oss/continuwuity
oss/continuwuity
1
0
Fork 0
mirror of https://forgejo.ellis.link/continuwuation/continuwuity.git synced 2025-09-30 18:42:05 +00:00
Code Wiki Activity

fix(hydra): Tackle most open code review comments

Browse source
This commit is contained in:
nexy7574 2025-09-11 20:41:48 +01:00
parent 40ebe37992
commit edb92f021b
No known key found for this signature in database
14 changed files with 105 additions and 124 deletions
Download patch file Download diff file Expand all files Collapse all files

9
src/api/client/membership/join.rs
View file

@ -553,20 +553,16 @@ async fn join_room_by_id_helper_remote(
.iter()
.stream()
.then(|pdu| {
debug!(?pdu, "Validating send_join response room_state event");
services
.server_keys
.validate_and_add_event_id_no_fetch(pdu, &room_version_id)
.inspect_err(|e| {
debug_warn!(
"Could not validate send_join response room_state event: {e:?}"
);
debug_warn!("Could not validate send_join response room_state event: {e:?}");
})
.inspect(|_| debug!("Completed validating send_join response room_state event"))
})
.ready_filter_map(Result::ok)
.fold(HashMap::new(), |mut state, (event_id, value)| async move {
debug!(?event_id, "Processing send_join response room_state event");
let pdu = match PduEvent::from_id_val(&event_id, value.clone()) {
| Ok(pdu) => pdu,
| Err(e) => {
@ -574,10 +570,8 @@ async fn join_room_by_id_helper_remote(
return state;
},
};
debug!(event_id = ?event_id.clone(), "Adding PDU outlier for send_join response room_state event");
services.rooms.outlier.add_pdu_outlier(&event_id, &value);
if let Some(state_key) = &pdu.state_key {
debug!(?state_key, "Creating shortstatekey for state event in send_join response");
let shortstatekey = services
.rooms
.short
@ -586,7 +580,6 @@ async fn join_room_by_id_helper_remote(
state.insert(shortstatekey, pdu.event_id.clone());
}
debug!("Completed send_join response");
state
})
.await;

12
src/api/client/membership/mod.rs
View file

@ -69,12 +69,11 @@ pub(crate) async fn banned_room_check(
}
if let Some(room_id) = room_id {
if services.rooms.metadata.is_banned(room_id).await
|| (room_id.server_name().is_some()
&& services
.moderation
.is_remote_server_forbidden(room_id.server_name().expect("legacy room mxid")))
{
let room_banned = services.rooms.metadata.is_banned(room_id).await;
let server_banned = room_id.server_name().is_some_and(|server_name| {
services.moderation.is_remote_server_forbidden(server_name)
});
if room_banned || server_banned {
warn!(
"User {user_id} who is not an admin attempted to send an invite for or \
attempted to join a banned room or banned room server name: {room_id}"
@ -107,7 +106,6 @@ pub(crate) async fn banned_room_check(
.boxed()
.await?;
}
return Err!(Request(Forbidden("This room is banned on this homeserver.")));
}
} else if let Some(server_name) = server_name {

42
src/api/client/room/create.rs
View file

@ -4,7 +4,7 @@ use axum::extract::State;
use conduwuit::{
Err, Result, RoomVersion, debug, debug_info, debug_warn, err, info,
matrix::{StateKey, pdu::PduBuilder},
warn,
trace, warn,
};
use conduwuit_service::{Services, appservice::RegistrationInfo};
use futures::FutureExt;
@ -82,12 +82,13 @@ pub(crate) async fn create_room_route(
};
let room_features = RoomVersion::new(&room_version)?;
let room_id: Option<OwnedRoomId> = match room_features.room_ids_as_hashes {
| true => None,
| false => match &body.room_id {
let room_id: Option<OwnedRoomId> = if !room_features.room_ids_as_hashes {
match &body.room_id {
| Some(custom_room_id) => Some(custom_room_id_check(&services, custom_room_id)?),
| None => Some(RoomId::new(services.globals.server_name())),
},
}
} else {
None
};
// check if room ID doesn't already exist instead of erroring on auth check
@ -179,7 +180,7 @@ pub(crate) async fn create_room_route(
| Some(room_id) => services.rooms.state.mutex.lock(&room_id).await,
| None => {
let temp_room_id = RoomId::new(services.globals.server_name());
debug_info!("Locking temporary room state mutex for {temp_room_id}");
trace!("Locking temporary room state mutex for {temp_room_id}");
services.rooms.state.mutex.lock(&temp_room_id).await
},
};
@ -202,12 +203,12 @@ pub(crate) async fn create_room_route(
)
.boxed()
.await?;
debug!("Created room create event with ID {}", create_event_id);
trace!("Created room create event with ID {}", create_event_id);
let room_id = match room_id {
| Some(room_id) => room_id,
| None => {
let as_room_id = create_event_id.as_str().replace('$', "!");
debug_info!("Creating room with v12 room ID {as_room_id}");
trace!("Creating room with v12 room ID {as_room_id}");
RoomId::parse(&as_room_id)?.to_owned()
},
};
@ -260,21 +261,26 @@ pub(crate) async fn create_room_route(
}
let mut creators: Vec<OwnedUserId> = vec![sender_user.to_owned()];
if let Some(additional_creators) = create_content.get("additional_creators") {
if let Some(additional_creators) = additional_creators.as_array() {
for creator in additional_creators {
if let Some(creator) = creator.as_str() {
if let Ok(creator) = OwnedUserId::parse(creator) {
creators.push(creator.clone());
users.insert(creator.clone(), int!(100));
// Do we care about additional_creators?
if room_features.explicitly_privilege_room_creators {
// Have they been specified?
if let Some(additional_creators) = create_content.get("additional_creators") {
// Are they a real array?
if let Some(additional_creators) = additional_creators.as_array() {
// Iterate through them
for creator in additional_creators {
// Are they a string?
if let Some(creator) = creator.as_str() {
// Do they parse into a real user ID?
if let Ok(creator) = OwnedUserId::parse(creator) {
// Add them to the power levels and creators
creators.push(creator.clone());
}
}
}
}
}
}
if !(RoomVersion::new(&room_version)?).explicitly_privilege_room_creators {
creators.clear();
}
let power_levels_content = default_power_levels_content(
body.power_level_content_override.as_ref(),

2
src/api/server/send.rs
View file

@ -138,7 +138,6 @@ async fn handle(
pdus: impl Stream<Item = Pdu> + Send,
edus: impl Stream<Item = Edu> + Send,
) -> Result<ResolvedMap> {
// TODO(hydra): Does having no room ID break this?
// group pdus by room
let pdus = pdus
.collect()
@ -187,7 +186,6 @@ async fn handle_room(
.lock(&room_id)
.await;
// TODO(hydra): We might be missing a room ID
let room_id = &room_id;
pdus.try_stream()
.and_then(|(_, event_id, value)| async move {

7
src/core/matrix/event/filter.rs
View file

@ -35,13 +35,16 @@ fn matches_room<E: Event>(event: &E, filter: &RoomEventFilter) -> bool {
if filter
.not_rooms
.iter()
.any(is_equal_to!(event.room_id().unwrap()))
.any(is_equal_to!(event.room_id().expect("event has a room ID")))
{
return false;
}
if let Some(rooms) = filter.rooms.as_ref() {
if !rooms.iter().any(is_equal_to!(event.room_id().unwrap())) {
if !rooms
.iter()
.any(is_equal_to!(event.room_id().expect("event has a room ID")))
{
return false;
}
}

20
src/core/matrix/pdu.rs
View file

@ -115,12 +115,20 @@ impl Event for Pdu {
#[inline]
fn room_id_or_hash(&self) -> OwnedRoomId {
if *self.event_type() != TimelineEventType::RoomCreate {
return self
.room_id()
.expect("Event must have a room ID")
.to_owned();
}
if let Some(room_id) = &self.room_id {
// v1-v11
room_id.clone()
} else {
// v12+
let constructed_hash = self.event_id.as_str().replace('$', "!");
RoomId::parse(&constructed_hash)
.expect("event ID can be indexed")
.expect("event ID can be parsed")
.to_owned()
}
}
@ -180,12 +188,20 @@ impl Event for &Pdu {
#[inline]
fn room_id_or_hash(&self) -> OwnedRoomId {
if *self.event_type() != TimelineEventType::RoomCreate {
return self
.room_id()
.expect("Event must have a room ID")
.to_owned();
}
if let Some(room_id) = &self.room_id {
// v1-v11
room_id.clone()
} else {
// v12+
let constructed_hash = self.event_id.as_str().replace('$', "!");
RoomId::parse(&constructed_hash)
.expect("event ID can be indexed")
.expect("event ID can be parsed")
.to_owned()
}
}

105
src/core/matrix/state_res/event_auth.rs
View file

@ -5,7 +5,7 @@ use futures::{
future::{OptionFuture, join, join3},
};
use ruma::{
Int, OwnedRoomId, OwnedUserId, RoomVersionId, UserId,
Int, OwnedUserId, RoomVersionId, UserId,
events::room::{
create::RoomCreateEventContent,
join_rules::{JoinRule, RoomJoinRulesEventContent},
@ -219,9 +219,8 @@ where
return Ok(false);
}
// TODO(hydra): If the create event has a room_id, reject
if room_version.room_ids_as_hashes && incoming_event.room_id().is_some() {
warn!("this room version does not support room IDs in m.room.create");
warn!("room create event incorrectly claims a room ID");
return Ok(false);
}
@ -239,7 +238,7 @@ where
return Ok(true);
}
// NOTE(hydra): We must have an event ID from this point forward.
// NOTE(hydra): We always have a room ID from this point forward.
/*
// TODO: In the past this code was commented as it caused problems with Synapse. This is no
@ -274,21 +273,6 @@ where
)
.await;
// TODO(hydra): Re-enable <v12 checks
// let room_create_event = match room_create_event {
// | None => {
// // Room was either v11 with no create event, or v12+ room
// if incoming_event.room_id().is_some() {
// // invalid v11
// warn!("no m.room.create event found in claimed state");
// return Ok(false);
// }
// // v12 room
// debug!("no m.room.create event found, assuming v12 room");
// create_event.clone()
// },
// | Some(e) => e,
// };
let room_create_event = create_event.clone();
// Get the content of the room create event, used later.
@ -301,14 +285,7 @@ where
warn!("invalid room version found in m.room.create event");
return Ok(false);
}
let expected_room_id = match room_version.room_ids_as_hashes {
// If the room version uses hashes, we replace the create event's event ID leading sigil
// with !
| true => OwnedRoomId::try_from(room_create_event.event_id().as_str().replace('$', "!"))
.expect("Failed to convert event ID to room ID")
.clone(),
| false => room_create_event.room_id().unwrap().to_owned(),
};
let expected_room_id = room_create_event.room_id_or_hash();
if incoming_event.room_id().unwrap() != expected_room_id {
warn!(
@ -347,17 +324,6 @@ where
}
}
// 3. If event does not have m.room.create in auth_events reject
// removed as part of Hydra.
// TODO: reintroduce this for <v12 lol
// if !incoming_event
// .auth_events()
// .any(|id| id == room_create_event.event_id())
// {
// warn!("no m.room.create event in auth events");
// return Ok(false);
// }
// If the create event content has the field m.federate set to false and the
// sender domain of the event does not match the sender domain of the create
// event, reject.
@ -471,12 +437,14 @@ where
if sender_member_event
.room_id()
.expect("we have a room ID for non create events")
!= room_create_event.room_id_or_hash()
!= expected_room_id
{
warn!(
"room_id of incoming event ({}) does not match room_id of m.room.create event ({})",
sender_member_event.room_id_or_hash(),
room_create_event.room_id_or_hash()
sender_member_event
.room_id()
.expect("event must have a room ID"),
expected_room_id
);
return Ok(false);
}
@ -650,6 +618,9 @@ where
E: Event + Send + Sync,
for<'a> &'a E: Event + Send,
{
fn is_creator(v: &RoomVersion, c: &BTreeSet<OwnedUserId>, user_id: &UserId) -> bool {
c.contains(user_id) && v.explicitly_privilege_room_creators
}
#[derive(Deserialize)]
struct GetThirdPartyInvite {
third_party_invite: Option<Raw<ThirdPartyInvite>>,
@ -719,7 +690,7 @@ where
let user_for_join_auth_is_valid = if let Some(user_for_join_auth) = user_for_join_auth {
// Is the authorised user allowed to invite users into this room
let (mut auth_user_pl, invite_level) = if let Some(pl) = &power_levels_event {
let (auth_user_pl, invite_level) = if let Some(pl) = &power_levels_event {
// TODO Refactor all powerlevel parsing
let invite =
deserialize_power_levels_content_invite(pl.content().get(), room_version)?.invite;
@ -735,19 +706,21 @@ where
} else {
(int!(0), int!(0))
};
if creators.contains(user_for_join_auth) {
auth_user_pl = Int::MAX;
}
(user_for_join_auth_membership == &MembershipState::Join)
&& (auth_user_pl >= invite_level)
let user_joined = user_for_join_auth_membership == &MembershipState::Join;
let okay_power = is_creator(room_version, &creators, user_for_join_auth)
|| auth_user_pl >= invite_level;
user_joined && okay_power
} else {
// No auth user was given
false
};
let sender_creator = is_creator(room_version, &creators, sender);
let target_creator = is_creator(room_version, &creators, target_user);
Ok(match target_membership {
| MembershipState::Join => {
debug!("starting target_membership=join check");
trace!("starting target_membership=join check");
// 1. If the only previous event is an m.room.create and the state_key is the
// creator,
// allow
@ -759,14 +732,8 @@ where
let no_more_prev_events = prev_events.next().is_none();
if prev_event_is_create_event && no_more_prev_events {
debug!("checking if sender is a room creator for initial membership event");
let is_creator = if room_version.explicitly_privilege_room_creators {
creators.contains(target_user) && creators.contains(sender)
} else if room_version.use_room_create_sender {
let creator = create_room.sender();
creator == sender && creator == target_user
} else {
trace!("checking if sender is a room creator for initial membership event");
let is_creator = (sender_creator && target_creator) || {
#[allow(deprecated)]
let creator = from_json_str::<RoomCreateEventContent>(create_room.content().get())?
.creator
@ -779,7 +746,7 @@ where
debug!("sender is room creator, allowing join");
return Ok(true);
}
debug!("sender is not room creator, proceeding with normal auth checks");
trace!("sender is not room creator, proceeding with normal auth checks");
}
let membership_allows_join = matches!(
target_user_current_membership,
@ -834,7 +801,7 @@ where
},
| MembershipState::Invite => {
// If content has third_party_invite key
debug!("starting target_membership=invite check");
trace!("starting target_membership=invite check");
match third_party_invite.and_then(|i| i.deserialize().ok()) {
| Some(tp_id) =>
if target_user_current_membership == MembershipState::Ban {
@ -865,9 +832,10 @@ where
);
false
} else {
let allow = sender_power
.filter(|&p| p >= &power_levels.invite)
.is_some();
let allow = sender_creator
|| sender_power
.filter(|&p| p >= &power_levels.invite)
.is_some();
if !allow {
warn!(
?target_user_membership_event_id,
@ -895,7 +863,8 @@ where
allow
} else if !sender_is_joined
|| target_user_current_membership == MembershipState::Ban
&& sender_power.filter(|&p| p < &power_levels.ban).is_some()
&& (sender_creator
|| sender_power.filter(|&p| p < &power_levels.ban).is_some())
{
warn!(
?target_user_membership_event_id,
@ -904,8 +873,9 @@ where
);
false
} else {
let allow = sender_power.filter(|&p| p >= &power_levels.kick).is_some()
&& target_power < sender_power;
let allow = sender_creator
|| (sender_power.filter(|&p| p >= &power_levels.kick).is_some()
&& target_power < sender_power);
if !allow {
warn!(
?target_user_membership_event_id,
@ -920,8 +890,9 @@ where
warn!(?sender_membership_event_id, "Can't ban user if sender is not joined");
false
} else {
let allow = sender_power.filter(|&p| p >= &power_levels.ban).is_some()
&& target_power < sender_power;
let allow = sender_creator
|| (sender_power.filter(|&p| p >= &power_levels.ban).is_some()
&& target_power < sender_power);
if !allow {
warn!(
?target_user_membership_event_id,
@ -986,6 +957,7 @@ where
/// Does the event have the correct userId as its state_key if it's not the ""
/// state_key.
fn can_send_event(event: &impl Event, ple: Option<&impl Event>, user_level: Int) -> bool {
// TODO(hydra): This function does not care about creators!
let event_type_power_level = get_send_level(event.event_type(), event.state_key(), ple);
debug!(
@ -1016,6 +988,7 @@ fn check_power_levels(
previous_power_event: Option<&impl Event>,
user_level: Int,
) -> Option<bool> {
// TODO(hydra): This function does not care about creators!
match power_event.state_key() {
| Some("") => {},
| Some(key) => {

2
src/service/rooms/auth_chain/mod.rs
View file

@ -195,7 +195,7 @@ async fn get_auth_chain_inner(
debug_error!(?event_id, ?e, "Could not find pdu mentioned in auth events");
},
| Ok(pdu) => {
if pdu.room_id.is_some() && pdu.room_id != Some(room_id.to_owned()) {
if pdu.room_id.as_ref().is_some_and(|r| r == room_id) {
return Err!(Request(Forbidden(error!(
?event_id,
?room_id,

2
src/service/rooms/event_handler/handle_incoming_pdu.rs
View file

@ -58,8 +58,6 @@ pub async fn handle_incoming_pdu<'a>(
value: BTreeMap<String, CanonicalJsonValue>,
is_timeline_event: bool,
) -> Result<Option<RawPduId>> {
// TODO(hydra): Room IDs should be calculated before this function is called
assert!(!room_id.is_empty(), "room ID cannot be empty");
// 1. Skip the PDU if we already have it as a timeline event
if let Ok(pdu_id) = self.services.timeline.get_pdu_id(event_id).await {
return Ok(Some(pdu_id));

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

@ -1,6 +1,6 @@
use std::{collections::HashSet, iter::once};
use conduwuit::{RoomVersion, debug_warn, trace};
use conduwuit::{RoomVersion, trace};
use conduwuit_core::{
Err, Result, implement,
matrix::{event::Event, pdu::PduBuilder},
@ -38,9 +38,7 @@ pub async fn build_and_append_pdu(
.await?;
let room_id = pdu.room_id_or_hash();
trace!("Checking if room {room_id} is an admin room");
if self.services.admin.is_admin_room(&room_id).await {
trace!("Room {room_id} is an admin room, checking PDU for admin room restrictions");
self.check_pdu_for_admin_room(&pdu, sender).boxed().await?;
}
@ -106,7 +104,6 @@ pub async fn build_and_append_pdu(
let room_features = RoomVersion::new(&content.room_version)?;
if room_features.room_ids_as_hashes {
// bootstrap shortid for room
debug_warn!(%room_id, "Bootstrapping shortid for room");
self.services
.short
.get_or_create_shortroomid(&room_id)

1
src/service/rooms/timeline/create.rs
View file

@ -72,7 +72,6 @@ pub async fn create_hash_and_sign_event(
};
let room_version = RoomVersion::new(&room_version_id).expect("room version is supported");
// TODO(hydra): Only create events can lack a room ID.
let prev_events: Vec<OwnedEventId> = match room_id {
| Some(room_id) =>

6
src/service/server_keys/get.rs
View file

@ -1,6 +1,6 @@
use std::borrow::Borrow;
use conduwuit::{Err, Result, debug, debug_error, implement};
use conduwuit::{Err, Result, debug_error, implement, trace};
use ruma::{
CanonicalJsonObject, RoomVersionId, ServerName, ServerSigningKeyId,
api::federation::discovery::VerifyKey,
@ -23,7 +23,7 @@ pub async fn get_event_keys(
return Err!(BadServerResponse("Failed to determine keys required to verify: {e}"));
},
};
debug!(?required, "Keys required to verify event");
trace!(?required, "Keys required to verify event");
let batch = required
.iter()
@ -73,7 +73,7 @@ pub async fn get_verify_key(
let notary_only = self.services.server.config.only_query_trusted_key_servers;
if let Some(result) = self.verify_keys_for(origin).await.remove(key_id) {
debug!("Found key in cache");
trace!("Found key in cache");
return Ok(result);
}

6
src/service/server_keys/mod.rs
View file

@ -8,7 +8,7 @@ mod verify;
use std::{collections::BTreeMap, sync::Arc, time::Duration};
use conduwuit::{
Result, Server, debug, debug_error, debug_warn, implement,
Result, Server, debug_error, debug_warn, implement, trace,
utils::{IterStream, timepoint_from_now},
};
use database::{Deserialized, Json, Map};
@ -120,12 +120,12 @@ pub async fn required_keys_exist(
) -> bool {
use ruma::signatures::required_keys;
debug!(?object, "Checking required keys exist");
trace!(?object, "Checking required keys exist");
let Ok(required_keys) = required_keys(object, version) else {
debug_error!("Failed to determine required keys");
return false;
};
debug!(?required_keys, "Required keys to verify event");
trace!(?required_keys, "Required keys to verify event");
required_keys
.iter()
.flat_map(|(server, key_ids)| key_ids.iter().map(move |key_id| (server, key_id)))

10
src/service/server_keys/verify.rs
View file

@ -1,5 +1,5 @@
use conduwuit::{
Err, Result, debug, debug_warn, implement, matrix::event::gen_event_id_canonical_json,
Err, Result, debug_warn, implement, matrix::event::gen_event_id_canonical_json, trace,
};
use ruma::{
CanonicalJsonObject, CanonicalJsonValue, OwnedEventId, RoomVersionId, signatures::Verified,
@ -30,9 +30,9 @@ pub async fn validate_and_add_event_id_no_fetch(
pdu: &RawJsonValue,
room_version: &RoomVersionId,
) -> Result<(OwnedEventId, CanonicalJsonObject)> {
debug!(?pdu, "Validating PDU without fetching keys");
trace!(?pdu, "Validating PDU without fetching keys");
let (event_id, mut value) = gen_event_id_canonical_json(pdu, room_version)?;
debug!(event_id = event_id.as_str(), "Generated event ID, checking required keys");
trace!(event_id = event_id.as_str(), "Generated event ID, checking required keys");
if !self.required_keys_exist(&value, room_version).await {
debug_warn!(
"Event {event_id} is missing required keys, cannot verify without fetching keys"
@ -41,14 +41,14 @@ pub async fn validate_and_add_event_id_no_fetch(
"Event {event_id} cannot be verified: missing keys."
)));
}
debug!("All required keys exist, verifying event");
trace!("All required keys exist, verifying event");
if let Err(e) = self.verify_event(&value, Some(room_version)).await {
debug_warn!("Event verification failed");
return Err!(BadServerResponse(debug_error!(
"Event {event_id} failed verification: {e:?}"
)));
}
debug!("Event verified successfully");
trace!("Event verified successfully");
value.insert("event_id".into(), CanonicalJsonValue::String(event_id.as_str().into()));