Skip to content

fix: dont send transaction to invited server #393

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ggazzo merged 1 commit into from
Mar 31, 2026
Merged

fix: dont send transaction to invited server #393

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions packages/federation-sdk/src/services/federation.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ export class FederationService {
});
}

async sendEventToAllServersInRoom(event: PersistentEventBase) {
async sendEventToAllServersInRoom(event: PersistentEventBase, omitDestinations: string[] = []): Promise<void> {
// TODO we need a map of rooms and destinations to avoid having to get rooms state just to send an event to all servers in the room.
const servers = await this.stateService.getServerSetInRoom(event.roomId);

Expand All @@ -205,17 +205,19 @@ export class FederationService {
}
}

// Sign the event once before queuing
await this.stateService.signEvent(event);

// Filter out the event origin and local server
const destinations = Array.from(servers).filter((server) => server !== event.origin && server !== this.configService.serverName);
// Filter out the event origin, local server, and any additional omitted destinations
const destinations = Array.from(servers).filter(
(server) => server !== event.origin && server !== this.configService.serverName && !omitDestinations.includes(server),
);

if (destinations.length === 0) {
this.logger.debug(`No destinations to send event ${event.eventId}`);
return;
}

// Sign the event once before queuing
await this.stateService.signEvent(event);

this.logger.info(`Queueing event ${event.eventId} for ${destinations.length} destinations`);

// Queue the event for all destinations
Expand Down
4 changes: 2 additions & 2 deletions packages/federation-sdk/src/services/invite.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ export class InviteService {
// can only invite if already part of the room
await stateService.handlePdu(PersistentEventFactory.createFromRawEvent(inviteResponse.event, roomVersion));

// let everyone know
void federationService.sendEventToAllServersInRoom(inviteEvent);
// let everyone else know
void federationService.sendEventToAllServersInRoom(inviteEvent, [invitedServer]);
Comment on lines +128 to +129
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Matrix federation v2 invite flow, when the inviting server relays the invite event to other room servers, must the relayed event include both inviter and invitee server signatures?

💡 Result:

Yes, in the Matrix federation v2 invite flow, when the inviting server relays the invite event to other room servers (via /send), the relayed event must include both the inviter server signature (origin server) and the invitee server signature.

Citations:

🏁 Script executed:

cat -n packages/federation-sdk/src/services/invite.service.ts | sed -n '115,145p'

Repository: RocketChat/homeserver

Length of output: 1168

🏁 Script executed:

cat -n packages/federation-sdk/src/services/invite.service.ts | sed -n '80,125p'

Repository: RocketChat/homeserver

Length of output: 1892

🏁 Script executed:

cat -n packages/federation-sdk/src/services/invite.service.ts | sed -n '50,100p'

Repository: RocketChat/homeserver

Length of output: 1979

🏁 Script executed:

cat -n packages/federation-sdk/src/services/invite.service.ts | sed -n '50,60p'

Repository: RocketChat/homeserver

Length of output: 428

Broadcast and return the doubly-signed invite event, not the single-signed one.

On Line 126 you persist inviteResponse.event, but on Lines 129 and 132-133 you broadcast and return inviteEvent. The SPEC comment on Line 97 explicitly requires the event to be "signed by both the inviting homeserver and the invited homeserver" before being sent to all servers. Broadcasting the single-signed inviteEvent violates the Matrix specification and will cause federation rejection.

🐛 Proposed fix 
-		await stateService.handlePdu(PersistentEventFactory.createFromRawEvent(inviteResponse.event, roomVersion));
+		const signedInviteEvent = PersistentEventFactory.createFromRawEvent(inviteResponse.event, roomVersion);
+		await stateService.handlePdu(signedInviteEvent);

 		// let everyone else know
-		void federationService.sendEventToAllServersInRoom(inviteEvent, [invitedServer]);
+		void federationService.sendEventToAllServersInRoom(signedInviteEvent, [invitedServer]);

 		return {
-			event_id: inviteEvent.eventId,
-			event: PersistentEventFactory.createFromRawEvent(inviteEvent.event, roomVersion),
+			event_id: signedInviteEvent.eventId,
+			event: signedInviteEvent,
 			room_id: roomId,
 		};
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/federation-sdk/src/services/invite.service.ts` around lines 128 -
129, The code is broadcasting and returning the singly-signed inviteEvent
instead of the doubly-signed inviteResponse.event that was persisted; change
uses of inviteEvent in federationService.sendEventToAllServersInRoom(...) and
the function return to use inviteResponse.event (the event returned from the
invited HS signing) so the broadcasted/returned event is the doubly-signed
version required by the SPEC; update any local variables or parameters that
expect inviteEvent accordingly (e.g., replace sending/returning inviteEvent with
inviteResponse.event and ensure inviteResponse.event is passed to
sendEventToAllServersInRoom and the function's return value).


return {
event_id: inviteEvent.eventId,
Expand Down
Loading