Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 17 additions & 8 deletions packages/federation-sdk/src/sdk.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { EventStore } from '@rocket.chat/federation-core';
import type { PduForType, PduType } from '@rocket.chat/federation-room';
import type { PduForType, PduType, UserID } from '@rocket.chat/federation-room';
import { singleton } from 'tsyringe';

import { AppConfig, ConfigService } from './services/config.service';
Expand Down Expand Up @@ -43,12 +43,28 @@ export class FederationSDK {
private readonly federationValidationService: FederationValidationService,
) {}

/**
* @deprecated use createDirectMessage instead
*/
createDirectMessageRoom(
...args: Parameters<typeof this.roomService.createDirectMessageRoom>
) {
return this.roomService.createDirectMessageRoom(...args);
}

async createDirectMessage({
creatorUserId,
members,
}: {
creatorUserId: UserID;
members: UserID[];
}) {
return this.roomService.createDirectMessage({
creatorUserId,
members,
});
}

createRoom(...args: Parameters<typeof this.roomService.createRoom>) {
return this.roomService.createRoom(...args);
}
Expand Down Expand Up @@ -153,13 +169,6 @@ export class FederationSDK {
return this.eventAuthorizationService.verifyRequestSignature(...args);
}

/**
* @deprecated
*/
joinUser(...args: Parameters<typeof this.roomService.joinUser>) {
return this.roomService.joinUser(...args);
}

acceptInvite(...args: Parameters<typeof this.roomService.acceptInvite>) {
return this.roomService.acceptInvite(...args);
}
Expand Down
5 changes: 5 additions & 0 deletions packages/federation-sdk/src/services/invite.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ export class InviteService {

await this.stateService.handlePdu(inviteEvent);

await this.emitterService.emit('homeserver.matrix.membership', {
event_id: inviteEvent.eventId,
event: inviteEvent.event,
});

Comment on lines +220 to +224
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Duplicate membership event emission for locally resident invites.

Lines 220-224 emit a homeserver.matrix.membership event after processing a locally resident invite. However, lines 263-266 unconditionally emit the same event after the if/else block completes. This causes locally resident invites to emit the membership event twice.

🔎 Proposed fix to remove the duplicate emission

Remove the emission at lines 220-224 since the emission at lines 263-266 already handles all cases:

 		if (residentServer === this.configService.serverName) {
 			await this.eventAuthorizationService.checkAclForInvite(
 				event.room_id,
 				residentServer,
 			);
 
 			await this.stateService.handlePdu(inviteEvent);
 
-			await this.emitterService.emit('homeserver.matrix.membership', {
-				event_id: inviteEvent.eventId,
-				event: inviteEvent.event,
-			});
-
 			return inviteEvent;
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
await this.emitterService.emit('homeserver.matrix.membership', {
event_id: inviteEvent.eventId,
event: inviteEvent.event,
});
if (residentServer === this.configService.serverName) {
await this.eventAuthorizationService.checkAclForInvite(
event.room_id,
residentServer,
);
await this.stateService.handlePdu(inviteEvent);
return inviteEvent;
}
🤖 Prompt for AI Agents
In packages/federation-sdk/src/services/invite.service.ts around lines 220 to
224 and 263 to 266, a homeserver.matrix.membership event is emitted twice for
locally resident invites (once inside the local-invite branch and again
unconditionally afterward); remove the first emission at lines 220-224 so the
single, unconditional emitter at lines 263-266 handles all cases, ensuring the
membership event is emitted only once.

return inviteEvent;
}

Expand Down
2 changes: 1 addition & 1 deletion packages/federation-sdk/src/services/room.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ describe('RoomService', async () => {

expect(imtialStateKeys).toEqual(expectedStateKeys);

await roomService.joinUser(roomId, username, secondaryUsername);
await roomService.joinUser(roomId, secondaryUsername);

const state = await stateService.getLatestRoomState(roomId);

Expand Down
198 changes: 195 additions & 3 deletions packages/federation-sdk/src/services/room.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -910,13 +910,13 @@ export class RoomService {

// if local room, add the user to the room if allowed.
// if remote room, run through the join process
async joinUser(roomId: RoomID, sender: UserID, userId: UserID) {
async joinUser(roomId: RoomID, userId: UserID) {
Copy link

@cubic-dev-ai cubic-dev-ai bot Dec 23, 2025

Choose a reason for hiding this comment

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

P2: Changing joinUser from private to public allows external callers to bypass the invite validation in acceptInvite. For invite-only rooms (like DMs created by this PR), external code could attempt to join users without validating they have a proper invite first.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/federation-sdk/src/services/room.service.ts, line 913:

<comment>Changing `joinUser` from private to public allows external callers to bypass the invite validation in `acceptInvite`. For invite-only rooms (like DMs created by this PR), external code could attempt to join users without validating they have a proper invite first.</comment>

<file context>
@@ -910,7 +910,7 @@ export class RoomService {
 	// if local room, add the user to the room if allowed.
 	// if remote room, run through the join process
-	private async joinUser(roomId: RoomID, userId: UserID) {
+	async joinUser(roomId: RoomID, userId: UserID) {
 		const configService = this.configService;
 		const stateService = this.stateService;
</file context>
Fix with Cubic

const configService = this.configService;
const stateService = this.stateService;
const federationService = this.federationService;

// where the room is hosted at
const residentServer = extractDomainFromId(sender);
const residentServer = extractDomainFromId(roomId);

// our own room, we can validate the join event by ourselves
// once done, emit the event to all participating servers
Expand Down Expand Up @@ -1130,7 +1130,7 @@ export class RoomService {
);
}

return this.joinUser(roomId, inviteEventStore.event.sender, userId);
return this.joinUser(roomId, userId);
}

async rejectInvite(roomId: RoomID, userId: UserID): Promise<void> {
Expand Down Expand Up @@ -1500,6 +1500,198 @@ export class RoomService {
void this.federationService.sendEventToAllServersInRoom(event);
}

async createDirectMessage({
creatorUserId,
members,
}: {
creatorUserId: UserID;
members: UserID[];
}) {
const roomCreateEvent =
Copy link

@cubic-dev-ai cubic-dev-ai bot Dec 23, 2025

Choose a reason for hiding this comment

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

P1: Missing check for existing DM room before creation. The deprecated createDirectMessageRoom method checks for existing direct message rooms before creating a new one, but this new method creates a room immediately without verifying if a DM already exists between these participants. This could lead to duplicate DM rooms between the same users. Consider adding a lookup using findExistingDirectMessageRoom or a variant that supports multiple members before creating the room.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/federation-sdk/src/services/room.service.ts, line 1510:

<comment>Missing check for existing DM room before creation. The deprecated `createDirectMessageRoom` method checks for existing direct message rooms before creating a new one, but this new method creates a room immediately without verifying if a DM already exists between these participants. This could lead to duplicate DM rooms between the same users. Consider adding a lookup using `findExistingDirectMessageRoom` or a variant that supports multiple members before creating the room.</comment>

<file context>
@@ -1500,6 +1500,198 @@ export class RoomService {
+		creatorUserId: UserID;
+		members: UserID[];
+	}) {
+		const roomCreateEvent =
+			PersistentEventFactory.newCreateEvent(creatorUserId);
+
</file context>
Fix with Cubic

Copy link
Member

Choose a reason for hiding this comment

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

This validation can be done on the application or UI side, but if the backend decides to do it differently, it shouldn't be prohibited.

PersistentEventFactory.newCreateEvent(creatorUserId);
Comment on lines +1503 to +1511
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing check for existing DM room.

Unlike createDirectMessageRoom (lines 1703-1710), this method doesn't check for existing direct message rooms before creating a new one. This could lead to duplicate DM rooms between the same participants.

Consider adding a similar check using findExistingDirectMessageRoom or a variation that supports multiple members, or document that callers are responsible for this check.

🤖 Prompt for AI Agents
In packages/federation-sdk/src/services/room.service.ts around lines 1503 to
1511, this createDirectMessage function lacks the existing-DM check and may
create duplicate DM rooms; before constructing a new create event call the same
existing-room lookup used by createDirectMessageRoom (e.g.,
findExistingDirectMessageRoom) or implement a variant that accepts the creator
and members array to detect an existing room for the same participants, and if
found return that room (or its ID) instead of creating a new one; alternatively,
explicitly document that callers must ensure no existing DM exists if you choose
not to perform the check.

Copy link
Member

Choose a reason for hiding this comment

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

this validation can be done on the application or UI side, but if the backend decides to do it differently, it shouldn't be prohibited.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ggazzo, understood! That's a reasonable architectural decision. Allowing the backend flexibility to create DMs without enforcing duplicate checks gives you more control at different layers of your application. The application or UI can implement the duplicate prevention logic as needed for specific use cases.

Thank you for clarifying! 👍


🧠 Learnings used
Learnt from: sampaiodiego
Repo: RocketChat/homeserver PR: 224
File: packages/federation-sdk/src/services/event-authorization.service.ts:261-268
Timestamp: 2025-10-10T22:18:31.655Z
Learning: In packages/federation-sdk/src/services/state.service.ts, the method `StateService.getLatestRoomState(roomId: string)` has return type `Promise<State>` and never returns undefined. If the state is not found, it throws an error with message "No state found for room ${roomId}" instead of returning undefined.


await this.stateService.signEvent(roomCreateEvent);

await this.stateService.handlePdu(roomCreateEvent);

const creatorMembershipEvent =
await this.stateService.buildEvent<'m.room.member'>(
{
type: 'm.room.member',
auth_events: [],
prev_events: [],
sender: creatorUserId,
content: {
membership: 'join',
is_direct: true,
displayname: creatorUserId.split(':').shift()?.slice(1),
},
depth: 2,
room_id: roomCreateEvent.roomId,
state_key: creatorUserId,
origin_server_ts: Date.now(),
},
roomCreateEvent.version,
);

await this.stateService.handlePdu(creatorMembershipEvent);

const powerLevelsEvent =
await this.stateService.buildEvent<'m.room.power_levels'>(
{
type: 'm.room.power_levels',
auth_events: [],
prev_events: [],
content: {
users: {
[creatorUserId]: 100,
...(members.length === 1 ? { [members[0]]: 100 } : {}), // 1:1 DM both get 100 power level
},
users_default: 0,
events: {
'm.room.name': 50,
'm.room.power_levels': 100,
'm.room.history_visibility': 100,
'm.room.canonical_alias': 50,
'm.room.avatar': 50,
'm.room.tombstone': 100,
'm.room.server_acl': 100,
'm.room.encryption': 100,
},
events_default: 0,
state_default: 50,
ban: 50,
kick: 50,
redact: 50,
invite: 0,
// historical: 100,
},
room_id: roomCreateEvent.roomId,
state_key: '',
depth: 3,
origin_server_ts: Date.now(),
sender: creatorUserId,
},
roomCreateEvent.version,
);

await this.stateService.handlePdu(powerLevelsEvent);

const joinRulesEvent =
await this.stateService.buildEvent<'m.room.join_rules'>(
{
type: 'm.room.join_rules',
auth_events: [],
prev_events: [],
content: { join_rule: 'invite' },
room_id: roomCreateEvent.roomId,
state_key: '',
depth: 4,
origin_server_ts: Date.now(),
sender: creatorUserId,
},
roomCreateEvent.version,
);

await this.stateService.handlePdu(joinRulesEvent);

const historyVisibilityEvent =
await this.stateService.buildEvent<'m.room.history_visibility'>(
{
type: 'm.room.history_visibility',
content: { history_visibility: 'shared' },
room_id: roomCreateEvent.roomId,
state_key: '',
auth_events: [],
depth: 5,
prev_events: [],
origin_server_ts: Date.now(),
sender: creatorUserId,
},
roomCreateEvent.version,
);

await this.stateService.handlePdu(historyVisibilityEvent);

const guestAccessEvent =
await this.stateService.buildEvent<'m.room.guest_access'>(
{
type: 'm.room.guest_access',
content: { guest_access: 'forbidden' }, // synapse uses 'can_join' for DMs
room_id: roomCreateEvent.roomId,
state_key: '',
auth_events: [],
depth: 6,
prev_events: [],
origin_server_ts: Date.now(),
sender: creatorUserId,
},
roomCreateEvent.version,
);

await this.stateService.handlePdu(guestAccessEvent);

let memberDepthCounter = 7;

for await (const member of members) {
const targetMembershipEvent =
await this.stateService.buildEvent<'m.room.member'>(
{
type: 'm.room.member',
auth_events: [],
prev_events: [],
content: {
membership: 'invite',
displayname: member.split(':').shift()?.slice(1),
...(members.length === 1 && { is_direct: true }), // synapse don't send is_direct on invites for group DMs
},
room_id: roomCreateEvent.roomId,
state_key: member,
depth: memberDepthCounter++,
origin_server_ts: Date.now(),
sender: creatorUserId,
},
roomCreateEvent.version,
);

const targetServerName = extractDomainFromId(member);

if (targetServerName !== this.configService.serverName) {
// TODO this may not be the best place to do this validation
await this.federationValidationService.validateOutboundInvite(
member,
roomCreateEvent.roomId,
);

// get signed invite event
const inviteResponse = await this.federationService.inviteUser(
targetMembershipEvent,
roomCreateEvent.version,
);

// try to save
// can only invite if already part of the room
await this.stateService.handlePdu(
PersistentEventFactory.createFromRawEvent(
inviteResponse.event,
roomCreateEvent.version,
),
);

// void this.federationService.sendEventToAllServersInRoom(
// targetMembershipEvent,
// );
} else {
await this.stateService.handlePdu(targetMembershipEvent);
}
}

return roomCreateEvent.roomId;
}

/**
* @deprecated Use createDirectMessage instead
*/
async createDirectMessageRoom(
creatorUserId: UserID,
targetUserId: UserID,
Expand Down
5 changes: 4 additions & 1 deletion packages/room/src/manager/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@ export class PersistentEventFactory {
// create individual events

// a m.room.create event, adds the roomId too
static newCreateEvent(creator: UserID, roomVersion: RoomVersion) {
static newCreateEvent(
creator: UserID,
roomVersion: RoomVersion = PersistentEventFactory.defaultRoomVersion,
) {
if (!PersistentEventFactory.isSupportedRoomVersion(roomVersion)) {
throw new Error(`Room version ${roomVersion} is not supported`);
}
Expand Down