fix(federation-matrix): skip avatar reset when avatar_url is missing

[Nainsi364]

Summary

Prevents federated user avatars from being reset when processing Matrix membership/join events that do not explicitly include avatar_url.

Closes #39909

Problem

Previously, when avatar_url was missing from a Matrix membership event, it was treated as null, causing Rocket.Chat to reset the user's avatar.

This is incorrect because:

• Matrix membership events may be stale
• The user may have updated their avatar later
• Processing older events could incorrectly remove the current avatar

Fix

Avatar updates are now processed only when the event explicitly includes the avatar_url field.

This ensures:

• Explicit avatar updates/removals are still handled
• Missing avatar_url no longer causes unintended avatar reset

Testing

• Verified avatar updates when avatar_url is present
• Verified avatar removal when avatar_url is explicitly null
• Verified no reset when avatar_url is missing

Summary by CodeRabbit

Bug Fixes

• Fixed an issue where avatar updates from remote servers were being processed even when no avatar data was available, preventing unnecessary operations.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 258677b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4aa02fe4-5957-4637-89db-ed094d14fa7c

📥 Commits

Reviewing files that changed from the base of the PR and between 4235cd9 and 258677b.

📒 Files selected for processing (1)

• ee/packages/federation-matrix/src/events/member.ts

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

• ee/packages/federation-matrix/src/events/member.ts

🧠 Learnings (6)

📓 Common learnings

Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.

📚 Learning: 2026-03-20T13:51:23.302Z

Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 39553
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts:179-181
Timestamp: 2026-03-20T13:51:23.302Z
Learning: In `apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts`, the truthiness guards `...(integration.avatar && { avatar })`, `...(integration.emoji && { emoji })`, `...(integration.alias && { alias })`, and `...(integration.script && { script })` in the `$set` payload of `updateIncomingIntegration` are intentional. Empty-string values for these fields should NOT overwrite the stored value — only truthy values are persisted. Do not flag these as bugs preventing explicit clears.

Applied to files:

• ee/packages/federation-matrix/src/events/member.ts

📚 Learning: 2025-10-28T16:53:42.761Z

Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

• ee/packages/federation-matrix/src/events/member.ts

📚 Learning: 2025-12-09T20:01:00.324Z

Learnt from: sampaiodiego
Repo: RocketChat/Rocket.Chat PR: 37532
File: ee/packages/federation-matrix/src/FederationMatrix.ts:920-927
Timestamp: 2025-12-09T20:01:00.324Z
Learning: When reviewing federation invite handling in Rocket.Chat (specifically under ee/packages/federation-matrix), understand that rejecting an invite via federationSDK.rejectInvite() triggers an event-driven cleanup: a leave event is emitted and handled by handleLeave() in ee/packages/federation-matrix/src/events/member.ts, which calls Room.performUserRemoval() to remove the subscription. Do not add explicit cleanup in the reject branch of handleInvite(); rely on the existing leave-event flow for cleanup. If making changes, ensure this invariant remains and that any related paths still funnel cleanup through the leave event to avoid duplicate or missing removals.

Applied to files:

• ee/packages/federation-matrix/src/events/member.ts

📚 Learning: 2026-02-26T19:25:44.063Z

Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

• ee/packages/federation-matrix/src/events/member.ts

📚 Learning: 2026-02-26T19:25:44.063Z

Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

• ee/packages/federation-matrix/src/events/member.ts

🔇 Additional comments (1)

ee/packages/federation-matrix/src/events/member.ts (1)

299-301: LGTM! Correctly handles the distinction between missing vs explicit avatar_url.

The 'avatar_url' in content check properly differentiates:

• Missing property (avatar_url not in event) → skips processing, preserving current avatar
• Explicit null (avatar_url: null) → resets avatar (intended behavior)
• Valid URL → updates avatar

This pattern aligns with the existing displayname handling on line 304.

---

Walkthrough

In the handleJoin function, the avatar-update logic now validates that the incoming Matrix content includes an avatar_url property before invoking the avatar update handler. Previously, the function would invoke the handler with content.avatar_url || null regardless of whether the property existed, potentially overwriting avatars with null when processing events without avatar data.

Changes

Cohort / File(s)	Summary
Avatar validation logic ee/packages/federation-matrix/src/events/member.ts	Added explicit check for avatar_url presence in content object before calling downloadAndSetAvatarDebounced, preventing unnecessary avatar updates when avatar data is absent from join events.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

type: bug, area: authentication

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely describes the main change: skipping avatar reset when avatar_url is missing in Matrix federation events.
Linked Issues check	✅ Passed	The code change addresses the core requirement from #39909 by distinguishing between explicit avatar removal (avatar_url present and null) and missing avatar_url, preventing unwanted avatar resets from stale events.
Out of Scope Changes check	✅ Passed	The changes are limited to the avatar update logic in the handleJoin function, directly addressing the linked issue without introducing unrelated modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[cubic-dev-ai]

No issues found across 1 file

[Nainsi364]

Hi, I’ve updated the PR title to follow the contribution guidelines. Please let me know if any further changes are needed.