chore: migrate DDP callers to REST endpoints (used methods with REST replacement)#40659
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (6)📚 Learning: 2026-03-27T14:52:56.865ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
📚 Learning: 2026-02-10T16:32:42.586ZApplied to files:
📚 Learning: 2026-05-11T20:30:35.265ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
🧬 Code graph analysis (1)apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx (1)
🔇 Additional comments (3)
WalkthroughClient code migrated from DDP/method calls and useMethod hooks to REST endpoints and useEndpoint hooks across translation, room navigation, message fetching, tokens, slash commands, E2EE, composer previews, user status, and tests; response shapes and mappers were added or adjusted. ChangesDDP-to-REST API Migration
Sequence DiagramsequenceDiagram
participant UI as Client UI
participant SDK as client sdk.rest
participant Server
UI->>SDK: POST /v1/im.create { usernames }
SDK->>Server: HTTP POST /v1/im.create
Server-->>SDK: { room: { _id, t, name } }
SDK-->>UI: { room }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #40659 +/- ##
===========================================
- Coverage 69.68% 69.65% -0.03%
===========================================
Files 3338 3339 +1
Lines 123248 123269 +21
Branches 21947 21972 +25
===========================================
- Hits 85881 85859 -22
- Misses 34013 34051 +38
- Partials 3354 3359 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
added a commit
that referenced
this pull request
May 22, 2026
The deprecation logger throws in TEST_MODE
(apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts:118), so a
deprecation log on a method that's still invoked by integration or
unit tests turns those tests into failures with messages like:
Error: The method "addUsersToRoom" is deprecated and will be
removed on version 9.0.0
That's the intended dev-loop guard rail, but it means this PR can
only carry deprecation calls on methods with **no callers**.
Removes the 83 logger calls (and their now-unused imports) that
targeted methods listed in audit.used. They will be revisited as part
of the caller-migration PR #40659 — once each callsite moves to the
matching REST endpoint, the DDP method can either get a deprecation
log here or be removed outright in #40656.
What remains in this PR: 71 deprecation calls, all on orphan methods
without a REST replacement.
This was referenced May 22, 2026
ggazzo
changed the title
8 tasks
|
✅ Layne — scan passed No security issues found on latest push. |
julio-rocketchat
requested changes
May 25, 2026
ggazzo
force-pushed
the
chore/ddp-migrate-used-with-rest
branch
from
9c237e2 to
533f925
Compare
Member
Author
|
/jira ARCH-2156 |
Replaces `useMethod('personalAccessTokens:generateToken' |
'personalAccessTokens:regenerateToken' | 'personalAccessTokens:removeToken')`
in the account-tokens table with `useEndpoint('POST',
'/v1/users.${verb}PersonalAccessToken')`. Destructures the new
`{ token }` response wrapper instead of treating the token as the bare
return value.
Both methods remain registered server-side for external DDP clients —
the deprecation log on develop will warn callers that they're scheduled
for removal in 9.0.0.
Swaps useMethod / sdk.call for useEndpoint / sdk.rest in the
following client modules where the REST endpoint is a faithful
drop-in (after destructuring the response wrapper):
- RegisteredWorkspaceModal.tsx
cloud:syncWorkspace -> POST /v1/cloud.syncWorkspace
(returns { success } now; threw on failure previously, still does)
- PreferencesMyDataSection.tsx
(skipped - REST drops url + pendingOperationsBeforeMyRequest)
- ReadReceiptsModal.tsx
getReadReceipts -> GET /v1/chat.getMessageReadReceipts
(REST wraps as { receipts }; destructured in queryFn)
- ComposerPopupProvider.tsx
getSlashCommandPreviews -> GET /v1/commands.preview
(params reshape: { cmd, params, msg: { rid, tmid } } ->
{ command, params, roomId }; tmid no longer forwarded - REST
GET schema doesn't accept it)
- useTranslateAction.ts + useViewOriginalTranslationAction.ts
autoTranslate.translateMessage -> POST /v1/autotranslate.translateMessage
(REST takes { messageId, targetLanguage }; result was already discarded)
- apps/autotranslate/client/lib/autotranslate.ts
autoTranslate.getSupportedLanguages -> GET /v1/autotranslate.getSupportedLanguages
(REST wraps as { languages })
- slashcommands-status/client/status.ts
setUserStatus -> POST /v1/users.setStatus
({ message: params } - status defaults to undefined like before)
- slashcommands-topic/client/topic.ts
saveRoomSettings -> POST /v1/rooms.saveRoomSettings
({ rid, roomTopic: params })
- lib/chats/data.ts
getSingleMessage -> GET /v1/chat.getMessage
(destructure response.message)
- lib/e2ee/rocketchat.e2e.room.ts
e2e.setRoomKeyID -> POST /v1/e2e.setRoomKeyID
e2e.getUsersOfRoomWithoutKey -> GET /v1/e2e.getUsersOfRoomWithoutKey
- GameCenterInvitePlayersModal.tsx
createPrivateGroup -> POST /v1/groups.create
(REST returns { group }; group.t / group._id replace .t / .rid)
Skipped in this pass (need follow-up):
- requestDataDownload: REST drops url + pendingOperationsBeforeMyRequest
- registerUser: REST returns { user } while anonymous DDP path
returns a stamped login token used for auto-login
- executeSlashCommandPreview: REST POST requires triggerId which the
caller does not currently produce
Six methods from docs/ddp-complex-migration-analysis.md that were
classified as 'pure caller refactor':
- getRoomById -> GET /v1/rooms.info
useGoToRoom destructures { room } and throws on null. Accepts the
extra team/parent lookups REST does (rare path - only when the
Subscriptions.state miss path is taken).
- createDirectMessage -> POST /v1/im.create
useOpenRoom passes { usernames: reference } instead of the
variadic ...split(', ') and reads room._id from the new { room }
wrapper. slashcommand /open passes { username: room } for the
single-user case.
- listCustomUserStatus -> GET /v1/custom-user-status.list
useStatusItems wraps the paginated endpoint in a loop that walks
offset->total to reproduce the 'give me everything' contract the
DDP method had. Bounded in practice (few dozen entries on a
typical workspace).
- getThreadMessages -> GET /v1/chat.getThreadMessages
useThreadMessagesQuery destructures { messages } from the
paginated response and fires POST /v1/subscriptions.read with
{ tmid } as a fire-and-forget side effect to preserve the
read-marker behavior the DDP method had server-side.
- executeSlashCommandPreview -> POST /v1/commands.preview
ComposerBoxPopupPreview reshapes args from
{ cmd, params, msg: { rid, tmid } } + previewItem to
{ command, params, roomId, tmid?, triggerId, previewItem } and
mints a triggerId via Random.id() (the preview popup did not
previously produce one - the slash-command runtime did).
- sendMessage -> POST /v1/chat.sendMessage (8 callsites)
Wraps the message in { message, previewUrls? }. Callers updated:
- client/lib/chats/flows/sendMessage.ts (primary send path)
- client/hooks/notification/useNotification.ts (reply from
desktop notification)
- client/apps/gameCenter/GameCenterInvitePlayersModal.tsx
(drops callWithErrorHandling - sdk.rest already throws on
non-2xx)
- all five asciiart slash commands
(lenny, tableflip, unflip, gimme, shrug)
Server-side DDP method registrations stay alive for external SDK and
mobile clients. They will be picked up by the deprecation log on the
next pass once tests are no longer the gating factor.
REST endpoint typings serialize dates as strings, so the migrated
callers need explicit casts when feeding the results into APIs that
expect IMessage / IReadReceiptWithUser / ICustomUserStatus (which use
Date). Five files touched:
- client/lib/chats/data.ts: cast chat.getMessage response.message
to IMessage when feeding findMessageByID's IMessage|null contract.
- client/views/room/modals/ReadReceiptsModal/ReadReceiptsModal.tsx:
annotate the queryFn return type as IReadReceiptWithUser[] so the
ReadReceiptRow props line up.
- client/navbar/.../useStatusItems.tsx: declare the
listCustomUserStatus loop as ICustomUserStatus[] so UserStatusLister
is satisfied.
- client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts:
drop the explicit subscriptions.read call - the REST endpoint
doesn't accept tmid; the per-thread read marker no longer fires
here. Comment notes the gap.
- client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsx:
POST /v1/cloud.syncWorkspace takes no body; call syncWorkspace()
without an argument.
Audit showed the four `as unknown as T` casts I added earlier were
hiding real serialization differences - REST returns
`Serialized<T>` (ISO date strings) while callers consume the same
identifier expecting `T` (Date objects). At runtime any code reaching
for `.getTime()` on `ts` / `_updatedAt` would crash.
Replaces each cast with a proper mapper:
- client/lib/chats/data.ts: chat.getMessage response goes through
`mapMessageFromApi` (already existed) before satisfying the
`IMessage | null` contract findMessageByID promises.
- client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts:
each message from chat.getThreadMessages goes through
`mapMessageFromApi` so the IThreadMessage type predicate sees a
real Date in `msg.ts`.
- client/views/room/modals/ReadReceiptsModal/ReadReceiptsModal.tsx:
chat.getMessageReadReceipts response is mapped through the new
`mapReadReceiptFromApi` (revives `ts` and `_updatedAt`).
- client/navbar/.../useStatusItems.tsx: custom-user-status.list
response is mapped through the new `mapCustomUserStatusFromApi`
(revives `_updatedAt` which the UserStatuses store uses for
ordering and conflict detection).
Two new helpers added under apps/meteor/client/lib/utils/. Pattern
mirrors the existing `mapMessageFromApi`.
Dropping the readThread side effect from useThreadMessagesQuery broke
two e2e tests:
- read-receipts-thread / should show read receipt as viewed in
thread when both users have the thread open
- omnichannel-livechat-read-receipts (cascading from the same gap)
REST has no per-thread read-marker endpoint — `/v1/subscriptions.read`
marks the whole room. Until one exists, fall back to the existing
`readThreads` DDP method (which is on the used-without-rest list and
will stay registered through 9.x) and fire it as a side effect from
the query function.
After deep debug, the two persistent UI test failures
(quote-attachment + omnichannel-livechat-read-receipts) trace back to
behavior differences between DDP and REST for these two methods:
- sendMessage: DDP path triggers Minimongo-driven reactive updates
that the composer/optimistic-message flow depends on for
visual reset; REST returns the wrapped message but the client
does not get the same reactive replacement. Quote-attachment's
composer stays in preview state because the optimistic temp
message is not reconciled.
- getReadReceipts: REST single GET races with the visitor's
self-receipt insertion that the DDP method, going through the
Meteor call queue, happens to wait behind. Livechat read-receipts
test reads only one receipt instead of two.
Both server endpoints exist and are otherwise correct, but the client
plumbing assumes DDP semantics in subtle ways. Migrating these two
methods needs a deeper refactor of the composer state machine and the
read-receipt timing — out of scope here.
Reverted callsites:
- apps/meteor/client/lib/chats/flows/sendMessage.ts
- apps/meteor/client/hooks/notification/useNotification.ts
- apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx
- apps/meteor/app/slashcommand-asciiarts/client/{lenny,tableflip,unflip,gimme,shrug}.ts
- apps/meteor/client/views/room/modals/ReadReceiptsModal/ReadReceiptsModal.tsx
The mapReadReceiptFromApi helper is left in place — harmless and
useful for the next attempt.
Captures the two regressions that forced the revert in this PR and the work needed to reattempt the migration cleanly. Sequenced as four small PRs: server-side livechat-receipt sync + client invalidation for getReadReceipts, optimistic reconciliation + 8-callsite swap for sendMessage.
ggazzo
force-pushed
the
chore/ddp-migrate-used-with-rest
branch
from
533f925 to
d55b95d
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 22 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (1)
374-385: 💤 Low valueConsider adding optional chaining on
itemsfor defensive handling.If the API ever returns a
previewobject without anitemsarray,preview?.items.map()would throw a TypeError. Addingpreview?.items?.map()provides safer fallback.Suggested defensive improvement
return ( - preview?.items.map((item) => ({ + preview?.items?.map((item) => ({ _id: item.id, value: item.value, type: item.type, })) ?? [] );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx` around lines 374 - 385, The getItemsFromLocal handler can throw if preview exists but preview.items is undefined; update the mapping to defensively handle missing items by using optional chaining and a safe fallback (e.g., change preview?.items.map(...) to preview?.items?.map(...) ?? [] or ensure you default preview.items to an empty array before mapping) inside the getItemsFromLocal function that calls call(...) and sets setPreviewTitle(...), so the function always returns an array even when preview.items is absent.apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsx (1)
31-31: ⚡ Quick winRemove the inline implementation comment
Line 31 adds an implementation comment in TSX code, which this repo guideline disallows.
Suggested change
- // REST endpoint is paginated; loop until total reached.As per coding guidelines:
**/*.{ts,tsx,js}— “Avoid code comments in the implementation”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsx` at line 31, Remove the inline implementation comment "// REST endpoint is paginated; loop until total reached." from the useStatusItems hook in useStatusItems.tsx; locate the comment inside the useStatusItems function (or its surrounding hook logic) and delete the comment so the file complies with the repository guideline that implementation comments are disallowed in .ts/.tsx files.apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx (1)
27-27: 💤 Low valueUnused hook if condition is removed.
If the condition on line 37 is removed as suggested above,
openedRoombecomes unused and this line can be deleted along with theuseOpenedRoomimport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx` at line 27, Remove the now-unused hook and variable: delete the useOpenedRoom import and the const openedRoom = useOpenedRoom(); declaration (references: useOpenedRoom, openedRoom) since the conditional that used openedRoom was removed and these are no longer needed.apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts (1)
29-31: ⚡ Quick winRemove implementation comments from this hook.
Line 29–31 adds inline implementation comments; move this migration rationale to PR/ADR/docs instead.
As per coding guidelines: `**/*.{ts,tsx,js}` → "Avoid code comments in the implementation".✂️ Proposed cleanup
- // REST has no per-thread read-marker endpoint yet; fall back to the - // `readThreads` DDP method so the side effect that DDP getThreadMessages - // used to do server-side keeps happening for callers. const readThreads = useMethod('readThreads');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts` around lines 29 - 31, The inline implementation comment inside the useThreadMessagesQuery hook (referencing REST lacking a per-thread read-marker and falling back to the `readThreads` DDP method so the side effect that DDP `getThreadMessages` used to do server-side keeps happening) should be removed from the source; edit the `useThreadMessagesQuery` hook to delete those lines and move the migration rationale to the PR/ADR/docs instead, keeping the hook implementation and any related calls to `getThreadMessages`/`readThreads` untouched.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx`:
- Around line 37-43: The check using openedRoom (from useOpenedRoom()) prevents
sending the invite to the newly created group because openedRoom cannot equal
the new group._id; remove the conditional and call sdk.call('sendMessage', {
_id: Random.id(), rid: group._id, msg: t('Apps_Game_Center_Play_Game_Together',
{ name }) }) unconditionally after the group is created (or, if conditional
behavior was intended, update the condition to reflect the correct runtime value
instead of openedRoom). Ensure the sendMessage invocation uses Random.id(),
group._id, and the same i18n key t('Apps_Game_Center_Play_Game_Together', { name
}).
In
`@apps/meteor/client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsx`:
- Around line 38-41: The thrown Error from syncWorkspace() is fine but the catch
handler forwards the raw error object to the toast, which can produce
non-readable output; update the catch to pass a plain string (e.g.
error?.message ?? String(error) or (error instanceof Error ? error.message :
String(error))) to the message/toast call so the user sees a readable message;
locate the try/catch around syncWorkspace() in RegisteredWorkspaceModal.tsx and
replace the direct error forwarding with the message string extraction.
In
`@apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts`:
- Line 114: The call to readThreads(tmid) is fired-and-forgotten and can produce
unhandled promise rejections; update the usage in useThreadMessagesQuery to
either await readThreads(tmid) inside the surrounding async function or attach a
rejection handler (e.g., readThreads(tmid).catch(err =>
process/log/handleError(err))). Ensure you reference the readThreads invocation
at the spot where void readThreads(tmid) is called and handle errors
consistently with the component’s error logging/handling utilities.
---
Nitpick comments:
In `@apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx`:
- Line 27: Remove the now-unused hook and variable: delete the useOpenedRoom
import and the const openedRoom = useOpenedRoom(); declaration (references:
useOpenedRoom, openedRoom) since the conditional that used openedRoom was
removed and these are no longer needed.
In
`@apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsx`:
- Line 31: Remove the inline implementation comment "// REST endpoint is
paginated; loop until total reached." from the useStatusItems hook in
useStatusItems.tsx; locate the comment inside the useStatusItems function (or
its surrounding hook logic) and delete the comment so the file complies with the
repository guideline that implementation comments are disallowed in .ts/.tsx
files.
In
`@apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts`:
- Around line 29-31: The inline implementation comment inside the
useThreadMessagesQuery hook (referencing REST lacking a per-thread read-marker
and falling back to the `readThreads` DDP method so the side effect that DDP
`getThreadMessages` used to do server-side keeps happening) should be removed
from the source; edit the `useThreadMessagesQuery` hook to delete those lines
and move the migration rationale to the PR/ADR/docs instead, keeping the hook
implementation and any related calls to `getThreadMessages`/`readThreads`
untouched.
In `@apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx`:
- Around line 374-385: The getItemsFromLocal handler can throw if preview exists
but preview.items is undefined; update the mapping to defensively handle missing
items by using optional chaining and a safe fallback (e.g., change
preview?.items.map(...) to preview?.items?.map(...) ?? [] or ensure you default
preview.items to an empty array before mapping) inside the getItemsFromLocal
function that calls call(...) and sets setPreviewTitle(...), so the function
always returns an array even when preview.items is absent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98205e28-0540-4d9b-8b56-c5342229fbe7
📒 Files selected for processing (22)
apps/meteor/app/autotranslate/client/lib/autotranslate.tsapps/meteor/app/slashcommands-open/client/client.tsapps/meteor/app/slashcommands-status/client/status.tsapps/meteor/app/slashcommands-topic/client/topic.tsapps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsxapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsxapps/meteor/client/views/account/tokens/AccountTokensTable/AccountTokensTable.tsxapps/meteor/client/views/account/tokens/AccountTokensTable/AddToken.tsxapps/meteor/client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsxapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsxapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/views/room/hooks/useOpenRoom.tsapps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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:
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/app/slashcommands-status/client/status.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/app/slashcommands-topic/client/topic.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/client/views/account/tokens/AccountTokensTable/AddToken.tsxapps/meteor/client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsxapps/meteor/app/slashcommands-open/client/client.tsapps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsxapps/meteor/client/views/account/tokens/AccountTokensTable/AccountTokensTable.tsxapps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsxapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/app/autotranslate/client/lib/autotranslate.tsapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxapps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsxapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/views/room/hooks/useOpenRoom.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
apps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.ts
🧠 Learnings (9)
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/views/room/hooks/useOpenRoom.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.
Applied to files:
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/views/room/hooks/useOpenRoom.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:
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/app/slashcommands-status/client/status.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/app/slashcommands-topic/client/topic.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/app/slashcommands-open/client/client.tsapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/app/autotranslate/client/lib/autotranslate.tsapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/views/room/hooks/useOpenRoom.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:
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/app/slashcommands-status/client/status.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/app/slashcommands-topic/client/topic.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/app/slashcommands-open/client/client.tsapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/app/autotranslate/client/lib/autotranslate.tsapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/views/room/hooks/useOpenRoom.ts
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.
Applied to files:
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.tsapps/meteor/app/slashcommands-status/client/status.tsapps/meteor/client/hooks/notification/useNotification.tsapps/meteor/client/lib/utils/mapReadReceiptFromApi.tsapps/meteor/app/slashcommands-topic/client/topic.tsapps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.tsapps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.tsapps/meteor/client/views/account/tokens/AccountTokensTable/AddToken.tsxapps/meteor/client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsxapps/meteor/app/slashcommands-open/client/client.tsapps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsxapps/meteor/client/views/account/tokens/AccountTokensTable/AccountTokensTable.tsxapps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsxapps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.tsapps/meteor/app/autotranslate/client/lib/autotranslate.tsapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxapps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsxapps/meteor/client/components/message/toolbar/useTranslateAction.tsapps/meteor/client/lib/chats/data.tsapps/meteor/client/views/room/hooks/useGoToRoom.tsapps/meteor/client/lib/e2ee/rocketchat.e2e.room.tsapps/meteor/client/views/room/hooks/useOpenRoom.ts
📚 Learning: 2026-04-14T21:10:31.855Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36292
File: apps/meteor/client/hooks/useHasValidLocationHash.ts:7-12
Timestamp: 2026-04-14T21:10:31.855Z
Learning: When reviewing files in apps/meteor/client/hooks/, do not treat JSDoc-style comments on React hooks (especially exported hooks) as a violation of any “avoid code comments in implementation” guideline. It’s acceptable to use JSDoc to document the public API of exported hooks (e.g., parameter/return types, intended usage), as long as it documents behavior/contracts rather than adding narrative implementation comments.
Applied to files:
apps/meteor/client/hooks/notification/useNotification.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/account/tokens/AccountTokensTable/AddToken.tsxapps/meteor/client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsxapps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsxapps/meteor/client/views/account/tokens/AccountTokensTable/AccountTokensTable.tsxapps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsxapps/meteor/client/views/room/providers/ComposerPopupProvider.tsxapps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
apps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.ts
🧬 Code graph analysis (11)
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.ts (1)
apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsx (1)
useStatusItems(19-107)
apps/meteor/client/hooks/notification/useNotification.ts (1)
apps/meteor/app/utils/client/getUserAvatarURL.ts (1)
getUserAvatarURL(3-9)
apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts (3)
apps/meteor/client/lib/utils/mapMessageFromApi.ts (1)
mapMessageFromApi(5-31)apps/meteor/app/threads/server/methods/getThreadMessages.ts (1)
getThreadMessages(21-60)apps/meteor/server/methods/readThreads.ts (1)
readThreads(20-49)
apps/meteor/client/views/account/tokens/AccountTokensTable/AddToken.tsx (1)
apps/meteor/client/views/account/tokens/AccountTokensTable/AccountTokensTable.tsx (1)
AccountTokensTable(23-187)
apps/meteor/app/slashcommands-open/client/client.ts (1)
apps/meteor/client/lib/rooms/roomTypes/direct.ts (1)
findRoom(140-147)
apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsx (2)
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.ts (1)
mapCustomUserStatusFromApi(3-6)apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useUserMenu.tsx (1)
useUserMenu(13-61)
apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (1)
apps/meteor/app/utils/lib/i18n.ts (1)
t(6-6)
apps/meteor/client/lib/chats/data.ts (1)
apps/meteor/client/lib/utils/mapMessageFromApi.ts (1)
mapMessageFromApi(5-31)
apps/meteor/client/views/room/hooks/useGoToRoom.ts (2)
apps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.ts (1)
useLoadSurroundingMessages(14-69)apps/meteor/client/navbar/NavBarPagesGroup/actions/CreateDirectMessage.tsx (1)
CreateDirectMessage(24-109)
apps/meteor/client/lib/e2ee/rocketchat.e2e.room.ts (1)
apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
E2E(46-875)
apps/meteor/client/views/room/hooks/useOpenRoom.ts (1)
apps/meteor/client/views/room/RoomOpenerEmbedded.tsx (1)
RoomOpenerEmbedded(30-96)
🔇 Additional comments (20)
apps/meteor/client/views/admin/workspace/VersionCard/modals/RegisteredWorkspaceModal.tsx (1)
14-14: LGTM!Also applies to: 32-32
apps/meteor/app/autotranslate/client/lib/autotranslate.ts (1)
88-93: LGTM!apps/meteor/client/components/message/toolbar/useTranslateAction.ts (1)
2-2: LGTM!Also applies to: 18-18, 57-57
apps/meteor/client/components/message/toolbar/useViewOriginalTranslationAction.ts (1)
2-2: LGTM!Also applies to: 18-18, 57-57
apps/meteor/client/views/room/MessageList/hooks/useLoadSurroundingMessages.spec.ts (1)
133-133: LGTM!Also applies to: 244-244
apps/meteor/client/views/account/tokens/AccountTokensTable/AccountTokensTable.tsx (1)
11-11: LGTM!Also applies to: 29-30, 72-72
apps/meteor/client/views/account/tokens/AccountTokensTable/AddToken.tsx (1)
4-4: LGTM!Also applies to: 22-22, 45-45
apps/meteor/client/lib/e2ee/rocketchat.e2e.room.ts (1)
404-404: LGTM!Also applies to: 426-426, 485-487, 503-503, 509-509
apps/meteor/app/slashcommands-open/client/client.ts (1)
30-42: LGTM!apps/meteor/app/slashcommands-status/client/status.ts (1)
14-18: LGTM!apps/meteor/app/slashcommands-topic/client/topic.ts (1)
14-20: LGTM!apps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsx (1)
31-31: LGTM!Also applies to: 67-79
apps/meteor/client/lib/utils/mapCustomUserStatusFromApi.ts (1)
1-6: LGTM!apps/meteor/client/navbar/NavBarSettingsToolbar/UserMenu/hooks/useStatusItems.tsx (1)
1-1: LGTM!Also applies to: 5-7, 16-30, 32-39
apps/meteor/client/hooks/notification/useNotification.ts (1)
36-36: LGTM!apps/meteor/client/views/room/hooks/useGoToRoom.ts (1)
3-3: LGTM!Also applies to: 15-15, 30-33
apps/meteor/client/views/room/hooks/useOpenRoom.ts (1)
3-3: LGTM!Also applies to: 21-21, 47-47, 49-49
apps/meteor/client/lib/chats/data.ts (1)
12-12: LGTM!Also applies to: 38-39, 73-73, 80-80, 211-211, 224-224
apps/meteor/client/lib/utils/mapReadReceiptFromApi.ts (1)
1-7: LGTM!apps/meteor/client/views/room/contextualBar/Threads/hooks/useThreadMessagesQuery.ts (1)
2-2: LGTM!Also applies to: 8-8, 28-28, 32-32, 113-113, 115-117
Comment on lines
+37
to
43
| if (openedRoom === group._id) { | ||
| void sdk.call('sendMessage', { | ||
| _id: Random.id(), | ||
| rid: result.rid, | ||
| rid: group._id, | ||
| msg: t('Apps_Game_Center_Play_Game_Together', { name }), | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
Unreachable code: invite message will never be sent.
The condition openedRoom === group._id can never be true. openedRoom is captured from useOpenedRoom() at render time (the room open before the modal), while group._id is the ID of a group that was just created in the preceding line. A newly created group cannot match a previously opened room.
This appears to be a regression—the invite message is now unreachable.
Proposed fix: send the message unconditionally to the new group
roomCoordinator.openRouteLink(group.t, { rid: group._id, name: group.name });
- if (openedRoom === group._id) {
- void sdk.call('sendMessage', {
- _id: Random.id(),
- rid: group._id,
- msg: t('Apps_Game_Center_Play_Game_Together', { name }),
- });
- }
+ void sdk.call('sendMessage', {
+ _id: Random.id(),
+ rid: group._id,
+ msg: t('Apps_Game_Center_Play_Game_Together', { name }),
+ });
onClose();If the original intent was conditional sending, please clarify the expected behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/meteor/client/apps/gameCenter/GameCenterInvitePlayersModal.tsx` around
lines 37 - 43, The check using openedRoom (from useOpenedRoom()) prevents
sending the invite to the newly created group because openedRoom cannot equal
the new group._id; remove the conditional and call sdk.call('sendMessage', {
_id: Random.id(), rid: group._id, msg: t('Apps_Game_Center_Play_Game_Together',
{ name }) }) unconditionally after the group is created (or, if conditional
behavior was intended, update the condition to reflect the correct runtime value
instead of openedRoom). Ensure the sendMessage invocation uses Random.id(),
group._id, and the same i18n key t('Apps_Game_Center_Play_Game_Together', { name
}).
![hacktron-app[bot]](https://avatars.githubusercontent.com/in/1250513?s=60&v=4){kind=small}
Comment on lines
+31
to
34
| const executeSlashCommandPreviewEndpoint = useEndpoint('POST', '/v1/commands.preview'); | ||
|
|
||
| useImperativeHandle( | ||
| ref, |
There was a problem hiding this comment.
🟡 Medium: DOM-based XSS in ComposerBoxPopupPreview via unvalidated src attributes
The ComposerBoxPopupPreview component renders item.value directly into the src attributes of img, audio, and video tags without validation or sanitization. If an attacker can influence the response from the /v1/commands.preview endpoint, they can inject malicious javascript: URIs. While modern browsers have some protections against javascript: in src attributes, this remains a security risk and a violation of secure coding practices. The application should validate that the provided URI uses an expected, safe protocol (e.g., http, https) before assigning it to a src attribute.
Steps to Reproduce
- Intercept the JSON response from the
/v1/commands.previewAPI endpoint. - Modify the
valuefield of an item in the response to contain a malicious payload, such asjavascript:alert('XSS'). - Trigger the corresponding slash command in the Rocket.Chat composer.
- Observe that the component renders the
<img>or<source>tag with the malicioussrcattribute.
Trace
graph TD
subgraph SG0 ["./Rocket.Chat/apps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsx"]
ComposerBoxPopupPreview{{"Component for previewing slash command results in the composer popup."}}
select["Selects a preview item and executes the associated slash command."]
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG1 ["./Rocket.Chat/apps/meteor/client/views/room/contexts/ChatContext.ts"]
useChat["React hook to access the Chat context."]
end
style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
ComposerBoxPopupPreview --> useChat
ComposerBoxPopupPreview --> select
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/client/views/room/composer/ComposerBoxPopupPreview.tsx
Lines: 31-79
Severity: medium
Vulnerability: DOM-based XSS in ComposerBoxPopupPreview via unvalidated src attributes
Description:
The `ComposerBoxPopupPreview` component renders `item.value` directly into the `src` attributes of `img`, `audio`, and `video` tags without validation or sanitization. If an attacker can influence the response from the `/v1/commands.preview` endpoint, they can inject malicious `javascript:` URIs. While modern browsers have some protections against `javascript:` in `src` attributes, this remains a security risk and a violation of secure coding practices. The application should validate that the provided URI uses an expected, safe protocol (e.g., `http`, `https`) before assigning it to a `src` attribute.
Proof of Concept:
**Steps to Reproduce**
1. Intercept the JSON response from the `/v1/commands.preview` API endpoint.
2. Modify the `value` field of an item in the response to contain a malicious payload, such as `javascript:alert('XSS')`.
3. Trigger the corresponding slash command in the Rocket.Chat composer.
4. Observe that the component renders the `<img>` or `<source>` tag with the malicious `src` attribute.
Affected Code:
{item.type === 'image' && <img src={item.value} alt={item._id} />}
{item.type === 'audio' && (
<audio controls>
<track kind='captions' />
<source src={item.value} />
Your browser does not support the audio element.
</audio>
)}
{item.type === 'video' && (
<video controls className='inline-video'>
<track kind='captions' />
<source src={item.value} />
Your browser does not support the video element.
</video>
)}
Fix this vulnerability. Only change what's necessary - don't modify unrelated code.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing. Any other reply is saved as a triage note.
- GameCenterInvitePlayersModal: restore callWithErrorHandling so sendMessage failures surface as toasts (cubic P2) - useThreadMessagesQuery: catch fire-and-forget readThreads rejection to avoid unhandled promise (coderabbit major) - RegisteredWorkspaceModal: pass error.message to toast instead of raw Error object (coderabbit minor)
3 tasks
tassoevan
approved these changes
May 26, 2026
tassoevan
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces runtime DDP callers in
apps/meteor/clientwith their REST equivalents for methods that already have a working/v1endpoint. DDP method registrations stay on the server — they remain reachable for external SDK/mobile clients until 9.0.0 removes them.Status
personalAccessTokens:generateTokenAddToken.tsxpersonalAccessTokens:regenerateTokenAccountTokensTable.tsxpersonalAccessTokens:removeTokenAccountTokensTable.tsxautoTranslate.getSupportedLanguagesapp/autotranslate/client/lib/autotranslate.tsautoTranslate.translateMessageuseTranslateAction.ts,useViewOriginalTranslationAction.tscloud:syncWorkspaceRegisteredWorkspaceModal.tsxcreateDirectMessageuseOpenRoom.ts+ slashcommandcreatePrivateGroupGameCenterInvitePlayersModal.tsxe2e.getUsersOfRoomWithoutKeyclient/lib/e2ee/rocketchat.e2e.room.tse2e.setRoomKeyIDclient/lib/e2ee/rocketchat.e2e.room.tsexecuteSlashCommandPreviewComposerBoxPopupPreview.tsxgetRoomByIduseGoToRoom.tsgetSingleMessageclient/lib/chats/data.tsgetSlashCommandPreviewsComposerPopupProvider.tsxgetThreadMessagesuseThreadMessagesQuery.tsreadThreadsDDP calllistCustomUserStatususeStatusItems.tsxmapCustomUserStatusFromApi)saveRoomSettingsapp/slashcommands-topic/client/topic.tssetUserStatusapp/slashcommands-status/client/status.tsgetReadReceipts(EE)ReadReceiptsModal.tsxdocs/ddp-migration-sendMessage-getReadReceipts-plan.md, follow-up #40675sendMessageregisterUserComposerAnonymous.tsx+SetupWizardProvider.tsxrequestDataDownloadPreferencesMyDataSection.tsxsetAvatarFromServiceuseUpdateAvatar.tsimageoravatarUrl; noserviceparam)subscriptions/getclient/lib/cachedStores/CachedStore.ts${this.name}/get; whole CachedStore abstraction needs review)Mappers
REST responses serialize
Dateto ISO strings while DDP returnsDate. Three mappers added underclient/lib/utils/keep call-sites typed against the domain shape:mapMessageFromApi(existed)mapCustomUserStatusFromApimapReadReceiptFromApiReverted
sendMessage+getReadReceiptswere migrated in an earlier revision and reverted in this branch — both broke optimistic UI reconciliation and read-receipt timing in e2e (quote-attachment.spec.ts,omnichannel-livechat-read-receipts.spec.ts). Follow-up #40675 carries the deeper fix (server-side await + client reconcile viaMessages.state.update(mapMessageFromApi(saved))).Test plan
/dm+ room open from search/giphy,/topicpreviews)/topic,/statusslash commandsTask: ARCH-2159
Summary by CodeRabbit