-
Notifications
You must be signed in to change notification settings - Fork 13k
chore(federation): accept and store encrypted messages #37191
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.
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
Changes from all commits
b0ab2cc
cb79ed7
23dcbd8
f974305
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,7 +38,7 @@ | |
| "@rocket.chat/core-services": "workspace:^", | ||
| "@rocket.chat/core-typings": "workspace:^", | ||
| "@rocket.chat/emitter": "^0.31.25", | ||
| "@rocket.chat/federation-sdk": "0.1.21", | ||
| "@rocket.chat/federation-sdk": "0.1.23", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify the SDK version for validity and security. Ensure the upgraded 🏁 Script executed: #!/bin/bash
# Description: Verify the federation-sdk version and check for security advisories
# Check npm registry for version existence
echo "Checking if @rocket.chat/[email protected] exists..."
npm view @rocket.chat/[email protected] version 2>/dev/null || echo "Version not found or registry unavailable"
# Check for security advisories using GitHub CLI
echo -e "\nChecking for security advisories..."
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "@rocket.chat/federation-sdk") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'Length of output: 185 Confirm @rocket.chat/[email protected] is published 🤖 Prompt for AI Agents |
||
| "@rocket.chat/http-router": "workspace:^", | ||
| "@rocket.chat/license": "workspace:^", | ||
| "@rocket.chat/models": "workspace:^", | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Validate both The handler only validates 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chainVerify edit handling for encrypted messages matches Matrix spec. The encrypted message edit handler doesn't check for If the Matrix specification requires - 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 - 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: 💡 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:
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
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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 };
}
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 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| emitter.on('homeserver.matrix.redaction', async (data) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const redactedEventId = data.redacts; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify the federation SDK version and check for security advisories.
The version bump from 0.1.21 to 0.1.23 aligns with the PR's dependency on homeserver PR #270. Ensure this version exists and has no known vulnerabilities.
Run the following script to verify the package version:
🏁 Script executed:
Length of output: 83
Fix the missing federation-sdk version reference
ee/packages/federation-matrix/package.jsonreferences"@rocket.chat/federation-sdk": "0.1.23", but version 0.1.23 isn’t published on npm. Publish v0.1.23 or revert to a published version.🤖 Prompt for AI Agents