Skip to content

fix: handle federation events for banned/invited users #384

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

Closed
ggazzo wants to merge 5 commits into from
Closed

fix: handle federation events for banned/invited users #384

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
270c3a0
fix: emit membership.rejected event when room state is missing for in…
ggazzo Mar 24, 2026
5565dde
docs: document server set routing bug after ban/unban cycle
ggazzo Mar 24, 2026
91e5cc4
fix: include banned and invited servers in event destination set
ggazzo Mar 24, 2026
50be08a
test: update getServerSetInRoom test to expect banned/invited servers
ggazzo Mar 24, 2026
288a7a2
docs: remove partial-room-state-for-invites doc
ggazzo Mar 27, 2026
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
4 changes: 4 additions & 0 deletions packages/federation-sdk/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@ export type HomeserverEventSignatures = {
room_id: string; // room where the change happened
role: 'moderator' | 'owner' | 'user'; // 50, 100, 0
};
'homeserver.matrix.membership.rejected': {
event: PduForType<'m.room.member'>;
reason: string;
};
};

export { roomIdSchema, userIdSchema, eventIdSchema, extractDomainFromId } from '@rocket.chat/federation-room';
Expand Down
19 changes: 13 additions & 6 deletions packages/federation-sdk/src/services/event.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,12 +167,19 @@ export class EventService {
try {
await this.validateEvent(event);
} catch (err) {
this.logger.error({
msg: 'Event validation failed',
origin,
event,
err,
});
if (err instanceof Error && err.name === 'UnknownRoomError' && event.type === 'm.room.member') {
await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
event,
reason: err.message,
});
Comment on lines +171 to +174
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P1: Do not let membership-rejection handler failures abort PDU processing; guard the emitted hook so validation failures still get skipped.

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/event.service.ts, line 171:

<comment>Do not let membership-rejection handler failures abort PDU processing; guard the emitted hook so validation failures still get skipped.</comment>

<file context>
@@ -167,12 +167,19 @@ export class EventService {
-							err,
-						});
+						if (err instanceof Error && err.name === 'UnknownRoomError' && event.type === 'm.room.member') {
+							await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
+								event,
+								reason: err.message,
</file context>
Suggested change
await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
event,
reason: err.message,
});
try {
await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
event,
reason: err.message,
});
} catch (emitErr) {
this.logger.error({
msg: 'Failed to emit membership rejection event',
origin,
event,
err: emitErr,
});
}
Fix with Cubic

} else {
Comment on lines +170 to +175
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard the rejection emit path so listener failures don’t abort PDU processing.

If eventEmitterService.emit(...) throws, this catch branch now bubbles and can fail the whole transaction. Validation failures here should remain fail-soft and continue.

💡 Suggested hardening 
-						if (err instanceof Error && err.name === 'UnknownRoomError' && event.type === 'm.room.member') {
-							await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
-								event,
-								reason: err.message,
-							});
+						if (err instanceof Error && err.name === 'UnknownRoomError' && event.type === 'm.room.member') {
+							try {
+								await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
+									event,
+									reason: err.message,
+								});
+							} catch (emitErr) {
+								this.logger.error({
+									msg: 'Failed to emit membership rejection event',
+									origin,
+									event,
+									err: emitErr,
+								});
+							}
 						} else {
📝 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 (err instanceof Error && err.name === 'UnknownRoomError' && event.type === 'm.room.member') {
await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
event,
reason: err.message,
});
} else {
if (err instanceof Error && err.name === 'UnknownRoomError' && event.type === 'm.room.member') {
try {
await this.eventEmitterService.emit('homeserver.matrix.membership.rejected', {
event,
reason: err.message,
});
} catch (emitErr) {
this.logger.error({
msg: 'Failed to emit membership rejection event',
origin,
event,
err: emitErr,
});
}
} else {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@packages/federation-sdk/src/services/event.service.ts` around lines 170 -
175, The UnknownRoomError branch currently calls
this.eventEmitterService.emit('homeserver.matrix.membership.rejected', ...)
directly which can throw and bubble up, aborting PDU processing; wrap that emit
call in its own try/catch so any exceptions from listeners are swallowed or
logged (e.g. via this.logger.warn/error) and do not rethrow, ensuring processing
continues; change the block inside the if that calls eventEmitterService.emit to
catch and handle errors locally while preserving the existing payload and
control flow.

this.logger.error({
msg: 'Event validation failed',
origin,
event,
err,
});
}
continue;
}

Expand Down
8 changes: 4 additions & 4 deletions packages/federation-sdk/src/services/state.service.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2262,7 +2262,7 @@ describe('StateService', async () => {
expect(servers.size).toBe(1);
});

it('should exclude servers with non-joined members', async () => {
it('should include servers with banned or invited members but exclude left', async () => {
const { roomCreateEvent } = await createRoom('public');

const creator = '@alice:example.com'; // Room creator with admin permissions
Expand All @@ -2284,9 +2284,9 @@ describe('StateService', async () => {

expect(servers.has('joined.com')).toBe(true);
expect(servers.has('left.com')).toBe(false);
expect(servers.has('banned.com')).toBe(false);
expect(servers.has('invited.com')).toBe(false);
expect(servers.size).toBe(2); // example.com (creator) + joined.com
expect(servers.has('banned.com')).toBe(true);
expect(servers.has('invited.com')).toBe(true);
expect(servers.size).toBe(4); // example.com (creator) + joined.com + banned.com + invited.com
});

it('should return creator server for room with only creator', async () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/federation-sdk/src/services/state.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,8 +697,10 @@ export class StateService {

const servers = new Set<string>();

const residentMemberships = new Set(['join', 'invite', 'ban']);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P1: getServerSetInRoom now includes ban/invite servers globally, which broadens EDU fanout and access authorization to non-joined servers.

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/state.service.ts, line 700:

<comment>`getServerSetInRoom` now includes `ban`/`invite` servers globally, which broadens EDU fanout and access authorization to non-joined servers.</comment>

<file context>
@@ -697,8 +697,10 @@ export class StateService {
 
 		const servers = new Set<string>();
 
+		const residentMemberships = new Set(['join', 'invite', 'ban']);
+
 		for (const event of state.values()) {
</file context>
Fix with Cubic


for (const event of state.values()) {
if (!event.isMembershipEvent() || event.getMembership() !== 'join') {
if (!event.isMembershipEvent() || !residentMemberships.has(event.getMembership() ?? '')) {
continue;
}

Expand Down
Loading