Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
13 changes: 11 additions & 2 deletions apps/meteor/server/services/messages/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,17 +88,22 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
async saveMessageFromFederation({
fromId,
rid,
msg,
federation_event_id,
msg,
e2e_content,
file,
files,
attachments,
thread,
}: {
fromId: string;
rid: string;
msg: string;
federation_event_id: string;
msg?: string;
e2e_content?: {
algorithm: string,
ciphertext: string,
},

Copilot AI Oct 9, 2025

Copy link

Choose a reason for hiding this comment

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

Inconsistent indentation: the properties inside e2e_content should use tabs to match the surrounding code style.

Suggested change
algorithm: string,
ciphertext: string,
},
algorithm: string,
ciphertext: string,
},

Copilot uses AI. Check for mistakes.
file?: IMessage['file'];
files?: IMessage['files'];
attachments?: IMessage['attachments'];
Expand All @@ -115,6 +120,10 @@ export class MessageService extends ServiceClassInternal implements IMessageServ
...(file && { file }),
...(files && { files }),
...(attachments && { attachments }),
...(e2e_content && {
t: 'e2e',
content: e2e_content,
}),
});
}

Expand Down
114 changes: 114 additions & 0 deletions ee/packages/federation-matrix/src/events/message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,120 @@ export function message(emitter: Emitter<HomeserverEventSignatures>, serverName:
}
});

emitter.on('homeserver.matrix.encrypted', async (data) => {
try {
if (!data.content.ciphertext) {
logger.debug('No message content found in event');
return;
}
Comment on lines +267 to +270

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate both algorithm and ciphertext fields.

The handler only validates ciphertext presence but not algorithm. Both fields are required for encrypted content according to the type definition in IMessageService.ts.

Apply this diff to validate both fields:

-		if (!data.content.ciphertext) {
-			logger.debug('No message content found in event');
+		if (!data.content.ciphertext || !data.content.algorithm) {
+			logger.debug('Missing required encrypted content fields (algorithm or ciphertext)');
 			return;
 		}
📝 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 (!data.content.ciphertext) {
logger.debug('No message content found in event');
return;
}
if (!data.content.ciphertext || !data.content.algorithm) {
logger.debug('Missing required encrypted content fields (algorithm or ciphertext)');
return;
}
🤖 Prompt for AI Agents
In ee/packages/federation-matrix/src/events/message.ts around lines 267 to 270,
the handler only checks for data.content.ciphertext but must validate both
data.content.ciphertext and data.content.algorithm; update the conditional to
verify both fields are present, log a clear debug message when either is missing
(e.g., "Missing encrypted message fields: algorithm or ciphertext") and return
early, ensuring encrypted content processing only proceeds when both algorithm
and ciphertext exist.


// at this point we know for sure the user already exists
const user = await Users.findOneByUsername(data.sender);
if (!user) {
throw new Error(`User not found for sender: ${data.sender}`);
}

const room = await Rooms.findOne({ 'federation.mrid': data.room_id });
if (!room) {
throw new Error(`No mapped room found for room_id: ${data.room_id}`);
}

const relation = data.content['m.relates_to'];

// SPEC: For example, an m.thread relationship type denotes that the event is part of a “thread” of messages and should be rendered as such.
const hasRelation = relation && 'rel_type' in relation;

const isThreadMessage = hasRelation && relation.rel_type === 'm.thread';

const threadRootEventId = isThreadMessage && relation.event_id;

// SPEC: Though rich replies form a relationship to another event, they do not use rel_type to create this relationship.
// Instead, a subkey named m.in_reply_to is used to describe the reply’s relationship,
const isRichReply = relation && !('rel_type' in relation) && 'm.in_reply_to' in relation;

const quoteMessageEventId = isRichReply && relation['m.in_reply_to']?.event_id;

const thread = threadRootEventId ? await getThreadMessageId(threadRootEventId) : undefined;

const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}
Comment on lines +300 to +314

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Verify edit handling for encrypted messages matches Matrix spec.

The encrypted message edit handler doesn't check for m.new_content (line 301), unlike the plaintext handler (line 155). For encrypted messages, verify whether edited content is provided in data.content['m.new_content'] or directly in data.content.

If the Matrix specification requires m.new_content for encrypted message edits as well, apply this diff:

-		if (isEditedMessage && relation.event_id) {
+		if (isEditedMessage && relation.event_id && data.content['m.new_content']) {
 			logger.debug('Received edited message from Matrix, updating existing message');

Additionally, the content comparison (line 311) should potentially compare against m.new_content:

-		if (originalMessage.content?.ciphertext === data.content.ciphertext) {
+		if (originalMessage.content?.ciphertext === data.content['m.new_content']?.ciphertext) {
 			logger.debug('No changes in message content, skipping update');

🌐 Web query:

Does the Matrix specification require m.new_content for encrypted message edits (m.room.encrypted with m.replace relation)?

💡 Result:

Short answer: Yes — the spec requires the replacement (edit) to include m.new_content, and for encrypted edits that m.new_content be inside the decrypted payload of the m.room.encrypted event. [1][2]

Notes/refs:

  • MSC2676 / message-editing: "The content of a message edit event must contain a m.new_content property..." and for encrypted replacements "m.new_content is placed in the content of the encrypted payload." [1]
  • The Client‑Server API / related spec discussion and issues note the practical/server-side complications (servers can't read encrypted payloads), but that is an implementation/aggregation concern — it does not remove the requirement that decrypted edit payloads contain m.new_content. [2][3]

Require m.new_content for encrypted edits

Apply the following changes in ee/packages/federation-matrix/src/events/message.ts (around lines 300–314):

-   if (isEditedMessage && relation.event_id) {
+   if (isEditedMessage && relation.event_id && data.content['m.new_content']) {
        logger.debug('Received edited message from Matrix, updating existing message');
...
-   if (originalMessage.content?.ciphertext === data.content.ciphertext) {
+   if (originalMessage.content?.ciphertext === data.content['m.new_content']?.ciphertext) {
        logger.debug('No changes in message content, skipping update');
📝 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
const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}
const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';
if (isEditedMessage && relation.event_id && data.content['m.new_content']) {
logger.debug('Received edited message from Matrix, updating existing message');
const originalMessage = await Messages.findOneByFederationId(relation.event_id);
if (!originalMessage) {
logger.error('Original message not found for edit:', relation.event_id);
return;
}
if (originalMessage.federation?.eventId !== relation.event_id) {
return;
}
if (originalMessage.content?.ciphertext === data.content['m.new_content']?.ciphertext) {
logger.debug('No changes in message content, skipping update');
return;
}


if (quoteMessageEventId) {
await Message.updateMessage(
{
...originalMessage,
content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
},
user,
originalMessage,
);
return;
}

await Message.updateMessage(
{
...originalMessage,
content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
},
user,
originalMessage,
);
return;
}

if (quoteMessageEventId) {
const originalMessage = await Messages.findOneByFederationId(quoteMessageEventId);
if (!originalMessage) {
logger.error('Original message not found for quote:', quoteMessageEventId);
return;
}
await Message.saveMessageFromFederation({
fromId: user._id,
rid: room._id,
e2e_content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
federation_event_id: data.event_id,
thread,
});
return;
}

await Message.saveMessageFromFederation({
fromId: user._id,
rid: room._id,
e2e_content: {
algorithm: data.content.algorithm,
ciphertext: data.content.ciphertext,
},
federation_event_id: data.event_id,
thread,
});
} catch (error) {
logger.error(error, 'Error processing Matrix message:');
}
});
Comment on lines +265 to +377

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Refactor duplicated logic into shared helper functions.

The encrypted message handler (lines 265-377) duplicates ~110 lines of logic from the plaintext handler (lines 115-263), including user/room resolution, relation parsing, thread handling, and quote detection. This violates DRY principles and increases maintenance burden.

Consider extracting shared logic into helper functions:

  1. Message context resolution (user, room, relations, threads):
async function resolveMessageContext(data: { sender: string; room_id: string; content: any }) {
	const user = await Users.findOneByUsername(data.sender);
	if (!user) {
		throw new Error(`User not found for sender: ${data.sender}`);
	}

	const room = await Rooms.findOne({ 'federation.mrid': data.room_id });
	if (!room) {
		throw new Error(`No mapped room found for room_id: ${data.room_id}`);
	}

	const relation = data.content['m.relates_to'];
	const hasRelation = relation && 'rel_type' in relation;
	const isThreadMessage = hasRelation && relation.rel_type === 'm.thread';
	const threadRootEventId = isThreadMessage && relation.event_id;
	const isRichReply = relation && !('rel_type' in relation) && 'm.in_reply_to' in relation;
	const quoteMessageEventId = isRichReply && relation['m.in_reply_to']?.event_id;
	const thread = threadRootEventId ? await getThreadMessageId(threadRootEventId) : undefined;
	const isEditedMessage = hasRelation && relation.rel_type === 'm.replace';

	return { user, room, relation, thread, quoteMessageEventId, isEditedMessage };
}
  1. Edit handling logic (extract common validation and update flow)

  2. Quote handling logic (extract common quote message resolution)

This would reduce the handlers to ~30-40 lines each, focusing only on content-specific logic (plaintext formatting vs. encrypted content handling).

🤖 Prompt for AI Agents
ee/packages/federation-matrix/src/events/message.ts around lines 115-377: the
encrypted handler (265-377) duplicates the plaintext handler (115-263) — extract
shared responsibilities into helpers: 1) create resolveMessageContext(data) that
finds user and room, parses relation, computes
hasRelation/isEditedMessage/isThreadMessage/threadRootEventId/thread (via
getThreadMessageId) and quoteMessageEventId, and returns { user, room, relation,
thread, quoteMessageEventId, isEditedMessage }; 2) create
handleEdit(originalMessage, data, user) that encapsulates the edit validation
(existence, federation id match, content equality) and performs
Message.updateMessage when needed, returning a boolean indicating it handled the
event; 3) create saveOrQuoteMessage({ userId, roomId, thread,
federation_event_id, e2e_content, quote }) that resolves quoted original message
if quote provided and calls Message.saveMessageFromFederation; then replace the
duplicated blocks in both handlers to call resolveMessageContext, use handleEdit
when isEditedMessage, and use saveOrQuoteMessage for quote/plain saves, keeping
original logging and try/catch semantics.


emitter.on('homeserver.matrix.redaction', async (data) => {
try {
const redactedEventId = data.redacts;
Expand Down
9 changes: 7 additions & 2 deletions packages/core-services/src/types/IMessageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,22 @@ export interface IMessageService {
saveMessageFromFederation({
fromId,
rid,
msg,
federation_event_id,
msg,
e2e_content,
file,
files,
attachments,
thread,
}: {
fromId: string;
rid: string;
msg: string;
federation_event_id: string;
msg?: string;
e2e_content?: {
algorithm: string,
ciphertext: string,
},

Copilot AI Oct 9, 2025

Copy link

Choose a reason for hiding this comment

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

Inconsistent indentation: the properties inside e2e_content should use tabs to match the surrounding code style.

Suggested change
algorithm: string,
ciphertext: string,
},
algorithm: string,
ciphertext: string,
},

Copilot uses AI. Check for mistakes.
file?: IMessage['file'];
files?: IMessage['files'];
attachments?: IMessage['attachments'];
Expand Down
Loading