fix: Ensure event_id is correctly stripped before verifying policy server signature

src/service/rooms/event_handler/policy_server.rs

@@ -47,11 +47,6 @@ pub(super) fn verify_policy_signature(
 ) -> bool {
 	#[cfg(debug_assertions)]
 	{
-		assert!(
-			!pdu_json.contains_key("event_id"),
-			"event_id should be removed from the JSON before verifying the policy server \
-			 signature"
-		);
 		let pretty = serde_json::to_string(pdu_json).unwrap();
 		trace!(data=%pretty, "Preparing to check policy server signature");
 	};
@@ -125,6 +120,10 @@ pub async fn policy_server_allows_event(
 	room_version_rules: &RoomVersionRules,
 	incoming: bool,
 ) -> Result<()> {
+	assert!(
+		!pdu_json.contains_key("event_id"),
+		"event_id should be removed from the JSON before calling policy_server_allows_event"
+	);
 	if pdu.event_type().with_state_key("") == (StateEventType::RoomPolicy, "".into()) {
 		return Ok(());
 	}
@@ -169,15 +168,7 @@ pub async fn policy_server_allows_event(
 	}
 
 	if incoming {
-		// Verify the signature instead of calling a check
+		if verify_policy_signature(&ps.via, ps_key, pdu_json, &room_version_rules.redaction) {
-		let event_id = pdu_json.remove("event_id");
-		let ps_allowed =
-			verify_policy_signature(&ps.via, ps_key, pdu_json, &room_version_rules.redaction);
-		if let Some(event_id) = event_id {
-			pdu_json.insert("event_id".into(), event_id);
-		}
-
-		if ps_allowed {
 			debug!(
 				via = %ps.via,
 				"Event is incoming and has a valid policy server signature"
@@ -195,7 +186,7 @@ pub async fn policy_server_allows_event(
 	if ps.via == self.services.globals.server_name()
 		&& !self.services.server.config.federation_loopback
 	{
-		warn!(
+		error!(
 			%ps.via,
 			%room_id,
 			"Cannot ask ourselves for a policy signature if `federation_loopback=false`",

src/service/rooms/event_handler/upgrade_outlier_pdu.rs

@@ -1,13 +1,14 @@
-use std::{borrow::Borrow, collections::BTreeMap, sync::Arc, time::Instant};
+use std::{borrow::Borrow, sync::Arc, time::Instant};
 
 use conduwuit::{
-	debug, debug_info, debug_warn, err, implement, is_equal_to, matrix::{state_res, Event, EventTypeExt, PduEvent, StateKey}, trace,
+	Err, Result, debug, debug_info, err, implement, info, is_equal_to,
+	matrix::{Event, EventTypeExt, PduEvent, StateKey, state_res},
+	trace,
 	utils::{
 		IterStream,
 		stream::{BroadbandExt, ReadyExt},
 	},
-	Err,
+	warn,
-	Result,
 };
 use futures::{FutureExt, StreamExt, future::ready};
 use ruma::{
@@ -250,6 +251,7 @@ where
 		// no reason to re-calculate that.
 		// 14-pre. ask the policy server to sign the event, if possible
 		debug!(event_id = %incoming_pdu.event_id, "Checking policy server for event");
+		let tmp_evt_id = val.remove("event_id");
 		if let Err(e) = self
 			.policy_server_allows_event(
 				&incoming_pdu,
@@ -277,6 +279,9 @@ where
 				"Event has passed policy server check."
 			);
 		}
+		if let Some(id) = tmp_evt_id {
+			val.insert("event_id".to_owned(), id);
+		}
 
 		// Additionally, if this is a redaction for a soft-failed event, we soft-fail it
 		// also.