Skip to content

fix(federation): duplicated events of room power_levels, name and topic #37174

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
Oct 8, 2025
Merged

fix(federation): duplicated events of room power_levels, name and topic #37174

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
10 changes: 7 additions & 3 deletions apps/meteor/app/channel-settings/server/functions/saveRoomName.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Message, Room } from '@rocket.chat/core-services';
import type { IUser } from '@rocket.chat/core-typings';
import { isRoomFederated } from '@rocket.chat/core-typings';
import { isRoomNativeFederated } from '@rocket.chat/core-typings';
import { Integrations, Rooms, Subscriptions } from '@rocket.chat/models';
import { Meteor } from 'meteor/meteor';
import type { Document, UpdateResult } from 'mongodb';
Expand Down Expand Up @@ -63,6 +63,10 @@ export async function saveRoomName(

await Room.beforeNameChange(room);

if (isRoomNativeFederated(room)) {
displayName = `${displayName}:${room.federation.mrid.split(':').pop()}`;
}
Comment on lines +66 to +68
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 8, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle undefined displayName before string concatenation.

The displayName parameter is typed as string | undefined, but the MRID-appending logic assumes it's defined. Additionally, room.federation.mrid.split(':').pop() can return undefined if the split produces an empty array, resulting in a string with "undefined" concatenated.

Apply this diff to safely handle undefined values:

 	await Room.beforeNameChange(room);
 
 	if (isRoomNativeFederated(room)) {
-		displayName = `${displayName}:${room.federation.mrid.split(':').pop()}`;
+		const serverPart = room.federation.mrid.split(':').pop();
+		if (displayName && serverPart) {
+			displayName = `${displayName}:${serverPart}`;
+		}
 	}
📝 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
if (isRoomNativeFederated(room)) {
displayName = `${displayName}:${room.federation.mrid.split(':').pop()}`;
}
await Room.beforeNameChange(room);
if (isRoomNativeFederated(room)) {
const serverPart = room.federation.mrid.split(':').pop();
if (displayName && serverPart) {
displayName = `${displayName}:${serverPart}`;
}
}
🤖 Prompt for AI Agents 
In apps/meteor/app/channel-settings/server/functions/saveRoomName.ts around
lines 66 to 68, the code assumes displayName and the MRID split result are
defined; instead guard against undefined by (1) checking displayName is a
non-empty string before concatenation (return or set a safe fallback like an
empty string if undefined), (2) safely derive the federation part by splitting
room.federation.mrid and selecting the last element only if it exists and is
non-empty (otherwise skip appending), and (3) perform the concatenation only
when both displayName and the federation part are valid to avoid producing
"undefined" in the final string.


if (displayName === room.name) {
return;
}
Expand All @@ -73,11 +77,11 @@ export async function saveRoomName(

const isDiscussion = Boolean(room?.prid);

const slugifiedRoomName = isDiscussion ? displayName : await getValidRoomName(displayName, rid);
const slugifiedRoomName = isDiscussion || isRoomNativeFederated(room) ? displayName : await getValidRoomName(displayName, rid);

let update;

if (isDiscussion || isRoomFederated(room)) {
if (isDiscussion || isRoomNativeFederated(room)) {
update = await updateFName(rid, displayName);
} else {
update = await updateRoomName(rid, displayName, slugifiedRoomName);
Expand Down
32 changes: 24 additions & 8 deletions ee/packages/federation-matrix/src/FederationMatrix.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,7 +838,12 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
throw new Error(`No Matrix room mapping found for room ${rid}`);
}

const userMui = isUserNativeFederated(user) ? user.federation.mui : `@${user.username}:${this.serverName}`;
if (isUserNativeFederated(user)) {
this.logger.debug('Only local users can change the name of a room, ignoring action');
return;
}

const userMui = `@${user.username}:${this.serverName}`;

await this.homeserverServices.room.updateRoomName(roomIdSchema.parse(room.federation.mrid), displayName, userIdSchema.parse(userMui));
}
Expand All @@ -854,7 +859,12 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
return;
}

const userMui = isUserNativeFederated(user) ? user.username : `@${user.username}:${this.serverName}`;
if (isUserNativeFederated(user)) {
this.logger.debug('Only local users can change the topic of a room, ignoring action');
return;
}

const userMui = `@${user.username}:${this.serverName}`;

await this.homeserverServices.room.setRoomTopic(roomIdSchema.parse(room.federation.mrid), userIdSchema.parse(userMui), topic);
}
Expand All @@ -874,18 +884,24 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
throw new Error('Leader role is not supported');
}

const userSender = await Users.findOneById(senderId);
if (!userSender) {
throw new Error(`No user found for ID ${senderId}`);
}

if (isUserNativeFederated(userSender)) {
this.logger.debug('Only local users can change roles of other users in a room, ignoring action');
return;
}

const senderMui = `@${userSender.username}:${this.serverName}`;

const user = await Users.findOneById(userId);
if (!user) {
throw new Error(`No user found for ID ${userId}`);
}
const userMui = isUserNativeFederated(user) ? user.federation.mui : `@${user.username}:${this.serverName}`;

const userSender = await Users.findOneById(senderId);
if (!userSender) {
throw new Error(`No user found for ID ${senderId}`);
}
const senderMui = isUserNativeFederated(userSender) ? userSender.federation.mui : `@${userSender.username}:${this.serverName}`;

let powerLevel = 0;
if (role === 'owner') {
powerLevel = 100;
Expand Down
Loading