Skip to content

fix(federation): handle "disinvite" and duplicated messages #37791

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 5 commits into from
Dec 15, 2025
Merged

fix(federation): handle "disinvite" and duplicated messages #37791

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1685260
chore: update @rocket.chat/federation-sdk to version 0.3.4 across mul…
ggazzo Dec 12, 2025
270e539
fix: handle errors when rejecting Matrix invites to ensure user remov…
ggazzo Dec 12, 2025
ed6c493
Create dull-tips-look.md
ggazzo Dec 12, 2025
01f9f99
test: federation add test for revoked invitation flow (#37790)
jessicaschelly Dec 12, 2025
266f381
chore: update @rocket.chat/federation-sdk to version 0.3.5 across mul…
ggazzo Dec 15, 2025
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
7 changes: 7 additions & 0 deletions .changeset/dull-tips-look.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/core-services": patch
"@rocket.chat/federation-matrix": patch
---

Fixes an issue where cases of invites that were canceled or disinvited were not being handled.
2 changes: 1 addition & 1 deletion apps/meteor/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
"@rocket.chat/emitter": "~0.31.25",
"@rocket.chat/favicon": "workspace:^",
"@rocket.chat/federation-matrix": "workspace:^",
"@rocket.chat/federation-sdk": "0.3.3",
"@rocket.chat/federation-sdk": "0.3.5",
"@rocket.chat/freeswitch": "workspace:^",
"@rocket.chat/fuselage": "^0.69.0",
"@rocket.chat/fuselage-forms": "~0.1.1",
Expand Down
2 changes: 1 addition & 1 deletion ee/packages/federation-matrix/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
"@rocket.chat/core-services": "workspace:^",
"@rocket.chat/core-typings": "workspace:^",
"@rocket.chat/emitter": "^0.31.25",
"@rocket.chat/federation-sdk": "0.3.3",
"@rocket.chat/federation-sdk": "0.3.5",
"@rocket.chat/http-router": "workspace:^",
"@rocket.chat/license": "workspace:^",
"@rocket.chat/models": "workspace:^",
Expand Down
12 changes: 10 additions & 2 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 @@ -8,7 +8,7 @@ import {
UserStatus,
} from '@rocket.chat/core-typings';
import type { MessageQuoteAttachment, IMessage, IRoom, IUser, IRoomNativeFederated } from '@rocket.chat/core-typings';
import { eventIdSchema, roomIdSchema, userIdSchema, federationSDK } from '@rocket.chat/federation-sdk';
import { eventIdSchema, roomIdSchema, userIdSchema, federationSDK, FederationRequestError } from '@rocket.chat/federation-sdk';
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

@rocket.chat/federation-sdk v0.3.4 FederationRequestError export response status

💡 Result:

I don’t see the v0.3.4 source online right now. In practice, FederationRequestError objects usually expose the HTTP status in one of these places — try checking in this order:

  • err.response?.status
  • err.statusCode
  • err.status

If that doesn’t work, log the error keys (console.log(Object.keys(err)) or console.dir(err, { depth: null })) and share the output and/or the thrown error stack and I’ll look up the SDK source (or fetch v0.3.4 on npm/GitHub) and give exact accessors.

🏁 Script executed:

# First, check the actual error handling in FederationMatrix.ts around lines 925-934
fd FederationMatrix.ts | head -5

Repository: RocketChat/Rocket.Chat

Length of output: 119

🏁 Script executed:

# Search for FederationRequestError usage in the codebase
rg "FederationRequestError" --type ts --type tsx -B 2 -A 5

Repository: RocketChat/Rocket.Chat

Length of output: 93

🏁 Script executed:

# Check package.json for federation-sdk version
rg "@rocket.chat/federation-sdk" package.json

Repository: RocketChat/Rocket.Chat

Length of output: 48

🏁 Script executed:

# Find the FederationMatrix.ts file and check lines around 925-934
fd FederationMatrix.ts -x cat -n {} | sed -n '920,940p'

Repository: RocketChat/Rocket.Chat

Length of output: 733

🏁 Script executed:

# Find package.json files that contain federation-sdk
fd package.json | xargs grep -l "federation-sdk"

Repository: RocketChat/Rocket.Chat

Length of output: 169

🏁 Script executed:

# Search for FederationRequestError with broader approach
rg "FederationRequestError" -A 5 -B 2

Repository: RocketChat/Rocket.Chat

Length of output: 1805

🏁 Script executed:

# Check the imports in FederationMatrix.ts
head -30 ee/packages/federation-matrix/src/FederationMatrix.ts

Repository: RocketChat/Rocket.Chat

Length of output: 1321

🏁 Script executed:

# Check federation-sdk version in ee/packages/federation-matrix/package.json
cat ee/packages/federation-matrix/package.json | grep -A 2 "federation-sdk"

Repository: RocketChat/Rocket.Chat

Length of output: 193

🏁 Script executed:

# Search for FederationRequestError class/interface definition in the codebase
rg "class FederationRequestError|interface FederationRequestError|type FederationRequestError" -A 10

Repository: RocketChat/Rocket.Chat

Length of output: 48

🏁 Script executed:

# Check if there's a node_modules or type definitions for federation-sdk
fd "FederationRequestError" --type f

Repository: RocketChat/Rocket.Chat

Length of output: 48

🏁 Script executed:

# Look for federation-sdk in node_modules or dist if available
find . -name "@rocket.chat" -type d 2>/dev/null | head -5

Repository: RocketChat/Rocket.Chat

Length of output: 48

🏁 Script executed:

# Search for any imports or usage of error.response in the codebase to see patterns
rg "error\.response" -A 2 -B 2 | head -40

Repository: RocketChat/Rocket.Chat

Length of output: 3444

🏁 Script executed:

# Check if there are other places where FederationRequestError is caught and used
rg "instanceof FederationRequestError" -B 5 -A 5

Repository: RocketChat/Rocket.Chat

Length of output: 1024

Use defensive chaining for error.response.status to match codebase patterns: Line 929 accesses error.response.status directly without a null/undefined check, while similar error handling elsewhere in the codebase (webdav, webdavClientAdapter) uses optional chaining (error.response?.statusText). Add a guard (error.response?.status === 403) or validate that error.response exists before accessing status.

🤖 Prompt for AI Agents 
In ee/packages/federation-matrix/src/FederationMatrix.ts around line 11 and
specifically where error.response.status is accessed (line ~929), the code
accesses error.response.status directly; change this to use defensive chaining
or a null check (e.g. error.response?.status === 403 or check that
error.response exists before reading status) to avoid runtime errors when
response is undefined, and mirror the existing pattern used elsewhere
(error.response?.statusText) so the conditional safely handles missing response
objects.

import type { EventID, UserID, FileMessageType, PresenceState } from '@rocket.chat/federation-sdk';
import { Logger } from '@rocket.chat/logger';
import { Users, Subscriptions, Messages, Rooms, Settings } from '@rocket.chat/models';
Expand Down Expand Up @@ -923,7 +923,15 @@ export class FederationMatrix extends ServiceClass implements IFederationMatrixS
await Room.performAcceptRoomInvite(room, subscription, user);
}
if (action === 'reject') {
await federationSDK.rejectInvite(room.federation.mrid, matrixUserId);
try {
await federationSDK.rejectInvite(room.federation.mrid, matrixUserId);
} catch (error) {
if (error instanceof FederationRequestError && error.response.status === 403) {
return Room.performUserRemoval(room, user);
}
this.logger.error(error, 'Failed to reject invite in Matrix');
throw error;
}
}
}
}
54 changes: 49 additions & 5 deletions ee/packages/federation-matrix/tests/end-to-end/room.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1647,18 +1647,62 @@ import { SynapseClient } from '../helper/synapse-client';
rid = pendingInvitation?.rid!;
}, 15000);

it('It should allow RC user to reject the invite', async () => {
it('should allow RC user to reject the invite and remove the subscription', async () => {
const rejectResponse = await rejectRoomInvite(rid, rc1AdminRequestConfig);
expect(rejectResponse.success).toBe(true);
});

it('It should remove the subscription after rejection', async () => {
const subscriptions = await getSubscriptions(rc1AdminRequestConfig);

const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));

expect(invitedSub).toBeFalsy();
});
Comment on lines +1650 to 1657
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 12, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid flakiness: wait/poll for subscription removal instead of asserting immediately.
If subscription cleanup is driven by async Matrix membership events, this can race. Prefer polling subscriptions.get until the INVITED sub disappears (or a timeout).

 it('should allow RC user to reject the invite and remove the subscription', async () => {
 	const rejectResponse = await rejectRoomInvite(rid, rc1AdminRequestConfig);
 	expect(rejectResponse.success).toBe(true);
 
-	const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
-	const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));
-	expect(invitedSub).toBeFalsy();
+	const deadline = Date.now() + 10_000;
+	while (true) {
+		const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
+		const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));
+		if (!invitedSub) break;
+		if (Date.now() > deadline) throw new Error('Timed out waiting for invited subscription removal');
+		await new Promise((r) => setTimeout(r, 250));
+	}
 });
📝 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
it('should allow RC user to reject the invite and remove the subscription', async () => {
const rejectResponse = await rejectRoomInvite(rid, rc1AdminRequestConfig);
expect(rejectResponse.success).toBe(true);
});
it('It should remove the subscription after rejection', async () => {
const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));
expect(invitedSub).toBeFalsy();
});
it('should allow RC user to reject the invite and remove the subscription', async () => {
const rejectResponse = await rejectRoomInvite(rid, rc1AdminRequestConfig);
expect(rejectResponse.success).toBe(true);
const deadline = Date.now() + 10_000;
while (true) {
const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));
if (!invitedSub) break;
if (Date.now() > deadline) throw new Error('Timed out waiting for invited subscription removal');
await new Promise((r) => setTimeout(r, 250));
}
});
🤖 Prompt for AI Agents 
In ee/packages/federation-matrix/tests/end-to-end/room.spec.ts around lines 1650
to 1657, the test asserts immediately that the invited subscription is removed
which can be flaky due to async Matrix membership events; replace the immediate
check with a polling/wait loop that repeatedly calls getSubscriptions (e.g.,
every 100–500ms) until the invited subscription is no longer present or a
sensible timeout (e.g., 5–10s) elapses, then assert that the invited
subscription is falsy; ensure the polling rejects/fails the test on timeout to
preserve test correctness.

});

describe('Revoked invitation flow from Synapse', () => {
describe('Synapse revokes an invitation before the RC user responds', () => {
let matrixRoomId: string;
let channelName: string;
let rid: string;

beforeAll(async () => {
channelName = `federated-channel-revoked-${Date.now()}`;
matrixRoomId = await hs1AdminApp.createRoom(channelName);

// hs1 invites RC user
await hs1AdminApp.matrixClient.invite(matrixRoomId, federationConfig.rc1.adminMatrixUserId);
const subscriptions = await getSubscriptions(rc1AdminRequestConfig);

const pendingInvitation = subscriptions.update.find(
(subscription) => subscription.status === 'INVITED' && subscription.fname?.includes(channelName),
);

expect(pendingInvitation).not.toBeUndefined();

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
rid = pendingInvitation?.rid!;

// hs1 revokes the invitation by kicking the invited user
await hs1AdminApp.matrixClient.kick(matrixRoomId, federationConfig.rc1.adminMatrixUserId, 'Invitation revoked');
}, 15000);

it('should fail when RC user tries to accept the revoked invitation', async () => {
const acceptResponse = await acceptRoomInvite(rid, rc1AdminRequestConfig);
expect(acceptResponse.success).toBe(false);
});

it('should allow RC user to reject the revoked invitation and remove the subscription', async () => {
const rejectResponse = await rejectRoomInvite(rid, rc1AdminRequestConfig);
expect(rejectResponse.success).toBe(true);

const subscriptions = await getSubscriptions(rc1AdminRequestConfig);
const invitedSub = subscriptions.update.find((sub) => sub.fname?.includes(channelName));
expect(invitedSub).toBeFalsy();
});

it('should have the RC user with leave membership on Synapse side after revoked invitation', async () => {
const member = await hs1AdminApp.findRoomMember(channelName, federationConfig.rc1.adminMatrixUserId);
expect(member?.membership).toBe('leave');
});
});
});
});
});
2 changes: 1 addition & 1 deletion packages/core-services/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
},
"dependencies": {
"@rocket.chat/core-typings": "workspace:^",
"@rocket.chat/federation-sdk": "0.3.3",
"@rocket.chat/federation-sdk": "0.3.5",
"@rocket.chat/http-router": "workspace:^",
"@rocket.chat/icons": "~0.45.0",
"@rocket.chat/media-signaling": "workspace:^",
Expand Down
14 changes: 7 additions & 7 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8284,7 +8284,7 @@ __metadata:
"@rocket.chat/apps-engine": "workspace:^"
"@rocket.chat/core-typings": "workspace:^"
"@rocket.chat/eslint-config": "workspace:^"
"@rocket.chat/federation-sdk": "npm:0.3.3"
"@rocket.chat/federation-sdk": "npm:0.3.5"
"@rocket.chat/http-router": "workspace:^"
"@rocket.chat/icons": "npm:~0.45.0"
"@rocket.chat/jest-presets": "workspace:~"
Expand Down Expand Up @@ -8495,7 +8495,7 @@ __metadata:
"@rocket.chat/ddp-client": "workspace:^"
"@rocket.chat/emitter": "npm:^0.31.25"
"@rocket.chat/eslint-config": "workspace:^"
"@rocket.chat/federation-sdk": "npm:0.3.3"
"@rocket.chat/federation-sdk": "npm:0.3.5"
"@rocket.chat/http-router": "workspace:^"
"@rocket.chat/license": "workspace:^"
"@rocket.chat/models": "workspace:^"
Expand All @@ -8521,9 +8521,9 @@ __metadata:
languageName: unknown
linkType: soft

"@rocket.chat/federation-sdk@npm:0.3.3":
version: 0.3.3
resolution: "@rocket.chat/federation-sdk@npm:0.3.3"
"@rocket.chat/federation-sdk@npm:0.3.5":
version: 0.3.5
resolution: "@rocket.chat/federation-sdk@npm:0.3.5"
dependencies:
"@datastructures-js/priority-queue": "npm:^6.3.5"
"@noble/ed25519": "npm:^3.0.0"
Expand All @@ -8536,7 +8536,7 @@ __metadata:
zod: "npm:^3.24.1"
peerDependencies:
typescript: ~5.9.2
checksum: 10/e93f0d59da8508ee0ecfb53f6d599f93130ab4b9f651d87da77615355261e4852d6f4659aae9f4c8881cf4b26a628512e6363ecc407d6e01a31a27d20b9d7684
checksum: 10/47de2265555649b375620c7a90cf84134b14f3e38d171d84276dee9196b7b303a2d8c6f693223d63ba1c464ce12c128c5f6a795c0bb42c0e5339587b2429d034
languageName: node
linkType: hard

Expand Down Expand Up @@ -9185,7 +9185,7 @@ __metadata:
"@rocket.chat/eslint-config": "workspace:^"
"@rocket.chat/favicon": "workspace:^"
"@rocket.chat/federation-matrix": "workspace:^"
"@rocket.chat/federation-sdk": "npm:0.3.3"
"@rocket.chat/federation-sdk": "npm:0.3.5"
"@rocket.chat/freeswitch": "workspace:^"
"@rocket.chat/fuselage": "npm:^0.69.0"
"@rocket.chat/fuselage-forms": "npm:~0.1.1"
Expand Down
Loading