chore: log deprecation for orphan DDP methods that have a REST replacement#40654
Merged
Conversation
Contributor
|
Looks like this PR is ready to merge! 🎉 |
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds calls to methodDeprecationLogger.method(...) at the start of numerous Meteor DDP method handlers, augments ServerMethods types for personal access token methods, and applies small optional-chaining/audit/notification tweaks. ChangesDDP Method Deprecation Initiative
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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. 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 #40654 +/- ##
===========================================
- Coverage 69.68% 69.64% -0.04%
===========================================
Files 3338 3339 +1
Lines 123248 123290 +42
Branches 21947 21972 +25
===========================================
- Hits 85881 85865 -16
- Misses 34013 34060 +47
- Partials 3354 3365 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
This was referenced May 22, 2026
11 tasks
ggazzo
changed the title
ggazzo
force-pushed
the
worktree-ddp-methods-removal
branch
from
5e0c00a to
51c93f6
Compare
Member
Author
|
/jira ARCH-2156 |
ggazzo
force-pushed
the
worktree-ddp-methods-removal
branch
from
51c93f6 to
1f90a39
Compare
scripts/audit-ddp-methods.mjs walks the monorepo to enumerate every
Meteor.methods registration, find their callers across server/client/
ee, and cross-reference against REST endpoints registered in
apps/meteor/app/api/server. Produces docs/ddp-audit.{json,md} with
three buckets: used methods, orphans (no caller), and used methods
without a REST replacement.
scripts/add-ddp-deprecation.mjs consumes that audit and inserts
methodDeprecationLogger.method(name, '9.0.0', replacement) into the
body of every orphan and every used-method-with-REST-replacement,
adding the import in the relative-imports group. Skips methods that
already log a deprecation. Supports shorthand, colon-function, arrow,
and twoFactorRequired-wrapped registration shapes.
Adds methodDeprecationLogger.method() calls to 148 Meteor methods that are either orphan (no caller found in the repo) or already have a REST endpoint replacement. Methods with the call now warn that they will be removed in 9.0.0, and when applicable point to the replacement endpoint. Methods still in use without a REST equivalent are intentionally left untouched until a replacement endpoint exists. Already-deprecated methods (sendFileMessage, starMessage, insertOrUpdateSound, uploadCustomSound) are kept as-is. Generated by scripts/add-ddp-deprecation.mjs from docs/ddp-audit.json.
The audit script picked up 'settings' and 'context' as method names from inside an object-literal return value and a JSDoc `@param` block. Those two methodDeprecationLogger calls were injected into the bodies of rocketchatSearch.getProvider and rocketchatSearch.search by mistake. Neither method is a deprecation target.
Initial pass shipped 18 deprecation-message replacements that pointed
developers at REST endpoints with materially different semantics or at
paths that did not exist. Independent review classified them as WRONG,
MISSING, or PARTIAL with silent-break risk:
WRONG (drop replacement, keep deprecation)
- 2fa:enable (TOTP enable vs email-2fa enable)
- 2fa:disable (TOTP disable vs send email code)
- deleteFileMessage (single-msg vs rooms.cleanHistory range purge)
- getMessages (batched IDs vs single chat.getMessage)
- loadNextMessages, loadSurroundingMessages
(forward/window fetch vs chat.syncMessages delta)
- readThreads (one thread vs whole-room read)
- getSetupWizardParameters (wizard subset vs settings.public)
- banner/dismiss (per-user set vs Banner.dismiss broadcast)
MISSING
- saveSettings (/v1/settings does not exist; only /v1/settings/:_id)
PARTIAL (room-type or shape mismatch — silent break for callers)
- loadHistory, joinRoom, leaveRoom, addUsersToRoom,
getRoomByTypeAndName (channels.* fails on p/d/l rooms)
- loadMissedMessages (chat.syncMessages has incompatible response)
- spotlight, slashCommand (REST drops required params)
For all 18, the replacement argument is now an empty array so the
deprecation logger still reports the removal version without pointing
to a wrong endpoint.
Also fills four mappings that the previous heuristic missed because of
case/separator differences or an unscanned EE api directory:
- autoTranslate.getSupportedLanguages
-> /v1/autotranslate.getSupportedLanguages
- autoTranslate.translateMessage
-> /v1/autotranslate.translateMessage
- subscriptions/get
-> /v1/subscriptions.get
- getReadReceipts (EE)
-> /v1/chat.getMessageReadReceipts
scripts/audit-ddp-methods.mjs gains:
- REST_DENYLIST that hard-blocks the 18 misleading mappings from being
re-derived if the heuristic runs again,
- lower-case and `/`-vs-`.` separator candidate paths so similarly
misspelled mappings get matched,
- apps/meteor/ee/server/api/ added to REST_DIRS so EE endpoints are
visible to the audit.
docs/ddp-replacement-review.md captures the per-method review notes.
Splits out the 87 deprecation-logger calls where the second argument was `[]` (no replacement endpoint). Those entries are still useful as runtime warnings, but they belong to a separate PR (chore/ddp-deprecation-no-replacement) so this PR focuses solely on methods whose callers can actually migrate to an existing /v1 endpoint today. After the strip, this PR keeps 66 method files instrumented with a real replacement path. The remaining files either matched origin/develop and were reverted, or had their methodDeprecationLogger import removed because no logger call survives. scripts/strip-empty-replacement-loggers.mjs is the helper that did the split — kept in tree so the inverse operation (verifying the no-replacement branch covers the methods dropped here) is reproducible.
Earlier strip pass dropped the 87 calls without a REST replacement. This pass takes the next slice: 24 methods that have a valid REST endpoint AND still have callers inside the monorepo. Those entries move out of this PR into a follow-up because they need their call sites refactored to hit the REST endpoint, not just a deprecation log. Methods moved out (need caller migration in follow-up PR): autoTranslate.getSupportedLanguages autoTranslate.translateMessage cloud:syncWorkspace createDirectMessage createPrivateGroup e2e.getUsersOfRoomWithoutKey e2e.setRoomKeyID executeSlashCommandPreview getReadReceipts getRoomById getSingleMessage getSlashCommandPreviews getThreadMessages listCustomUserStatus personalAccessTokens:generateToken personalAccessTokens:regenerateToken personalAccessTokens:removeToken registerUser requestDataDownload saveRoomSettings sendMessage setAvatarFromService setUserStatus subscriptions/get What remains in this PR: 43 orphan methods that already have a REST replacement. Each logs a deprecation pointing at the existing /v1 endpoint so any external DDP client still calling them gets a clear migration target. The unused methodDeprecationLogger imports left behind by the strip were removed in the same pass.
… proxy These 5 methods were classified as orphan by the static-string caller audit but are actually invoked at runtime from API tests through the `/v1/method.call/:method` REST proxy. Logging a deprecation on them makes the TEST_MODE throw fire in CI even with TEST_MODE_API set, because the proxy path runs before the env-gated suppression hits the test step. Removes the logger calls and now-unused imports for: - cleanRoomHistory - getUsersOfRoom - readMessages - setUserActiveStatus - updateMessage These methods are removed entirely in #40656 (release-9.0.0); deprecation logging on develop is therefore redundant. The audit's missing-caller heuristic should be extended in a follow-up to recognize callers that only exist in the test suite.
ggazzo
force-pushed
the
worktree-ddp-methods-removal
branch
from
1f90a39 to
1ca1a95
Compare
![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: 7
🤖 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/app/lib/server/methods/saveSettings.ts`:
- Around line 133-134: In saveSettings, guard access to this.connection before
reading clientAddress and httpHeaders: replace direct reads of
this.connection.clientAddress and this.connection.httpHeaders['user-agent'] with
optional-chaining and safe fallback (e.g. this.connection?.clientAddress ?? ''
and this.connection?.httpHeaders?.['user-agent'] ?? '') so IP and user-agent are
safely handled when connection is undefined.
In `@apps/meteor/server/methods/hideRoom.ts`:
- Line 36: The deprecation mapping for hideRoom is incorrect: update the call to
methodDeprecationLogger.method('hideRoom', '9.0.0', '/v1/rooms.leave') to
reference the correct API route '/v1/rooms.hide' since hideRoom calls
Subscriptions.hideByRoomIdAndUserId (not removeUserFromRoom); locate the
methodDeprecationLogger.method invocation in hideRoom and change the third
argument to '/v1/rooms.hide'.
In `@apps/meteor/server/methods/removeUserFromRoom.ts`:
- Line 132: The deprecation log call in removeUserFromRoom only emits
'/v1/channels.kick' regardless of room type; update the
methodDeprecationLogger.method invocation to pick the correct deprecation route
based on room.t (e.g., compute a route variable like route = room.t === 'c' ?
'/v1/channels.kick' : '/v1/groups.kick' and pass that route to
methodDeprecationLogger.method('removeUserFromRoom', '9.0.0', route)) so both
public channels and private groups are logged correctly.
In `@scripts/add-ddp-deprecation.mjs`:
- Around line 26-33: Validate the parsed argv values for the IIFE-produced only
and limit variables: for the only assignment (where argv is checked for
'--only'), ensure the extracted value is one of the permitted plan names
(compare against your canonical list, e.g. PLAN_NAMES or allowedPlans) and if
not print a clear error and exit non-zero; for the limit assignment (where argv
is checked for '--limit'), ensure Number(argv[i+1]) yields a finite positive
integer (use Number.isFinite and > 0 or Number.isInteger as appropriate) and if
not print a clear error and exit non-zero; apply the same validation logic to
the duplicate parsing block later in the file (the similar code around lines
199–214) so invalid inputs do not produce a silent no-op.
In `@scripts/audit-ddp-methods.mjs`:
- Around line 451-453: The code does not guard against a missing value after the
--out flag so outDir can become undefined and cause mkdirSync(outDir, ...) to
throw; update the logic around argv, outIdx and outDir to verify argv[outIdx +
1] exists and is not another flag (e.g., startsWith('--')) before using it,
otherwise fall back to the default join(ROOT, 'docs') or throw a clear error;
ensure mkdirSync only runs when outDir is a valid string.
- Around line 181-244: The value-skipping loop (variables body, p, q and the
loop that ends with i = q) is treating nested object keys as top-level keys; fix
it by introducing a top-level depth counter and only treat ',' or '}' as
terminators when that top-level depth is zero—i.e., increment/decrement depth on
encountering '{' or '}' (and also handle '(' and '[' as you already do for
nested groups), and continue skipping everything (strings, comments, template
expressions) until the top-level depth returns to zero; update the loop that
currently breaks on ',' or '}' so it checks that depth === 0 before breaking and
then set i = q as before.
In `@scripts/strip-empty-replacement-loggers.mjs`:
- Around line 50-53: stripLoggerImport currently skips removing the import only
when ANY_CALL.test(src) (which looks for `.method(`), causing valid uses like
property access of methodDeprecationLogger to lose imports; update
stripLoggerImport to also detect direct identifier or member usages of the
logger (e.g., test for /\bmethodDeprecationLogger\b/ or other logger names used
elsewhere) before removing the import (use IMPORT_RE to remove only when neither
ANY_CALL.test(src) nor a logger-identifier regex matches), referencing the
stripLoggerImport function, ANY_CALL, IMPORT_RE, and the methodDeprecationLogger
identifier to locate and implement the check.
🪄 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: 5c978795-cbf2-4b79-a467-8a836e4f13bc
📒 Files selected for processing (45)
apps/meteor/app/discussion/server/methods/createDiscussion.tsapps/meteor/app/e2e/server/methods/fetchMyKeys.tsapps/meteor/app/e2e/server/methods/setUserPublicAndPrivateKeys.tsapps/meteor/app/importer/server/methods/downloadPublicImportFile.tsapps/meteor/app/importer/server/methods/getImportFileData.tsapps/meteor/app/importer/server/methods/getImportProgress.tsapps/meteor/app/importer/server/methods/getLatestImportOperations.tsapps/meteor/app/importer/server/methods/startImport.tsapps/meteor/app/importer/server/methods/uploadImportFile.tsapps/meteor/app/lib/server/methods/archiveRoom.tsapps/meteor/app/lib/server/methods/createChannel.tsapps/meteor/app/lib/server/methods/deleteUserOwnAccount.tsapps/meteor/app/lib/server/methods/getChannelHistory.tsapps/meteor/app/lib/server/methods/saveSetting.tsapps/meteor/app/lib/server/methods/saveSettings.tsapps/meteor/app/lib/server/methods/sendMessage.tsapps/meteor/app/lib/server/methods/setEmail.tsapps/meteor/app/lib/server/methods/setRealName.tsapps/meteor/app/lib/server/methods/unarchiveRoom.tsapps/meteor/app/mentions/server/methods/getUserMentionsByChannel.tsapps/meteor/app/message-mark-as-unread/server/unreadMessages.tsapps/meteor/app/message-pin/server/pinMessage.tsapps/meteor/app/statistics/server/methods/getStatistics.tsapps/meteor/app/threads/server/methods/followMessage.tsapps/meteor/app/threads/server/methods/getThreadsList.tsapps/meteor/app/threads/server/methods/unfollowMessage.tsapps/meteor/imports/personal-access-tokens/server/api/methods/generateToken.tsapps/meteor/imports/personal-access-tokens/server/api/methods/regenerateToken.tsapps/meteor/imports/personal-access-tokens/server/api/methods/removeToken.tsapps/meteor/server/methods/channelsList.tsapps/meteor/server/methods/deleteUser.tsapps/meteor/server/methods/getRoomNameById.tsapps/meteor/server/methods/hideRoom.tsapps/meteor/server/methods/ignoreUser.tsapps/meteor/server/methods/messageSearch.tsapps/meteor/server/methods/removeUserFromRoom.tsapps/meteor/server/methods/resetAvatar.tsapps/meteor/server/methods/toggleFavorite.tsdocs/ddp-audit.jsondocs/ddp-audit.mddocs/ddp-deprecation-plan.jsondocs/ddp-replacement-review.mdscripts/add-ddp-deprecation.mjsscripts/audit-ddp-methods.mjsscripts/strip-empty-replacement-loggers.mjs
💤 Files with no reviewable changes (4)
- apps/meteor/app/lib/server/methods/sendMessage.ts
- apps/meteor/imports/personal-access-tokens/server/api/methods/generateToken.ts
- apps/meteor/imports/personal-access-tokens/server/api/methods/regenerateToken.ts
- apps/meteor/imports/personal-access-tokens/server/api/methods/removeToken.ts
📜 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)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (1/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.0 coverage (4/5)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.0 (4/4)
🧰 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:
apps/meteor/server/methods/resetAvatar.tsapps/meteor/server/methods/toggleFavorite.tsapps/meteor/app/e2e/server/methods/fetchMyKeys.tsapps/meteor/app/importer/server/methods/startImport.tsapps/meteor/app/threads/server/methods/followMessage.tsapps/meteor/server/methods/getRoomNameById.tsapps/meteor/server/methods/removeUserFromRoom.tsapps/meteor/app/importer/server/methods/getImportProgress.tsapps/meteor/app/message-pin/server/pinMessage.tsapps/meteor/app/e2e/server/methods/setUserPublicAndPrivateKeys.tsapps/meteor/app/lib/server/methods/archiveRoom.tsapps/meteor/app/lib/server/methods/saveSettings.tsapps/meteor/server/methods/channelsList.tsapps/meteor/app/mentions/server/methods/getUserMentionsByChannel.tsapps/meteor/app/message-mark-as-unread/server/unreadMessages.tsapps/meteor/app/importer/server/methods/uploadImportFile.tsapps/meteor/app/importer/server/methods/downloadPublicImportFile.tsapps/meteor/app/lib/server/methods/unarchiveRoom.tsapps/meteor/app/statistics/server/methods/getStatistics.tsapps/meteor/app/lib/server/methods/createChannel.tsapps/meteor/app/threads/server/methods/unfollowMessage.tsapps/meteor/app/importer/server/methods/getImportFileData.tsapps/meteor/app/lib/server/methods/getChannelHistory.tsapps/meteor/app/threads/server/methods/getThreadsList.tsapps/meteor/server/methods/messageSearch.tsapps/meteor/app/lib/server/methods/setRealName.tsapps/meteor/server/methods/hideRoom.tsapps/meteor/server/methods/ignoreUser.tsapps/meteor/server/methods/deleteUser.tsapps/meteor/app/lib/server/methods/saveSetting.tsapps/meteor/app/lib/server/methods/deleteUserOwnAccount.tsapps/meteor/app/importer/server/methods/getLatestImportOperations.tsapps/meteor/app/lib/server/methods/setEmail.tsapps/meteor/app/discussion/server/methods/createDiscussion.ts
🧠 Learnings (4)
📚 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/server/methods/resetAvatar.tsapps/meteor/server/methods/toggleFavorite.tsapps/meteor/app/e2e/server/methods/fetchMyKeys.tsapps/meteor/app/importer/server/methods/startImport.tsapps/meteor/app/threads/server/methods/followMessage.tsapps/meteor/server/methods/getRoomNameById.tsapps/meteor/server/methods/removeUserFromRoom.tsapps/meteor/app/importer/server/methods/getImportProgress.tsapps/meteor/app/message-pin/server/pinMessage.tsapps/meteor/app/e2e/server/methods/setUserPublicAndPrivateKeys.tsapps/meteor/app/lib/server/methods/archiveRoom.tsapps/meteor/app/lib/server/methods/saveSettings.tsapps/meteor/server/methods/channelsList.tsapps/meteor/app/mentions/server/methods/getUserMentionsByChannel.tsapps/meteor/app/message-mark-as-unread/server/unreadMessages.tsapps/meteor/app/importer/server/methods/uploadImportFile.tsapps/meteor/app/importer/server/methods/downloadPublicImportFile.tsapps/meteor/app/lib/server/methods/unarchiveRoom.tsapps/meteor/app/statistics/server/methods/getStatistics.tsapps/meteor/app/lib/server/methods/createChannel.tsapps/meteor/app/threads/server/methods/unfollowMessage.tsapps/meteor/app/importer/server/methods/getImportFileData.tsapps/meteor/app/lib/server/methods/getChannelHistory.tsapps/meteor/app/threads/server/methods/getThreadsList.tsapps/meteor/server/methods/messageSearch.tsapps/meteor/app/lib/server/methods/setRealName.tsapps/meteor/server/methods/hideRoom.tsapps/meteor/server/methods/ignoreUser.tsapps/meteor/server/methods/deleteUser.tsapps/meteor/app/lib/server/methods/saveSetting.tsapps/meteor/app/lib/server/methods/deleteUserOwnAccount.tsapps/meteor/app/importer/server/methods/getLatestImportOperations.tsapps/meteor/app/lib/server/methods/setEmail.tsapps/meteor/app/discussion/server/methods/createDiscussion.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/server/methods/resetAvatar.tsapps/meteor/server/methods/toggleFavorite.tsapps/meteor/app/e2e/server/methods/fetchMyKeys.tsapps/meteor/app/importer/server/methods/startImport.tsapps/meteor/app/threads/server/methods/followMessage.tsapps/meteor/server/methods/getRoomNameById.tsapps/meteor/server/methods/removeUserFromRoom.tsapps/meteor/app/importer/server/methods/getImportProgress.tsapps/meteor/app/message-pin/server/pinMessage.tsapps/meteor/app/e2e/server/methods/setUserPublicAndPrivateKeys.tsapps/meteor/app/lib/server/methods/archiveRoom.tsapps/meteor/app/lib/server/methods/saveSettings.tsapps/meteor/server/methods/channelsList.tsapps/meteor/app/mentions/server/methods/getUserMentionsByChannel.tsapps/meteor/app/message-mark-as-unread/server/unreadMessages.tsapps/meteor/app/importer/server/methods/uploadImportFile.tsapps/meteor/app/importer/server/methods/downloadPublicImportFile.tsapps/meteor/app/lib/server/methods/unarchiveRoom.tsapps/meteor/app/statistics/server/methods/getStatistics.tsapps/meteor/app/lib/server/methods/createChannel.tsapps/meteor/app/threads/server/methods/unfollowMessage.tsapps/meteor/app/importer/server/methods/getImportFileData.tsapps/meteor/app/lib/server/methods/getChannelHistory.tsapps/meteor/app/threads/server/methods/getThreadsList.tsapps/meteor/server/methods/messageSearch.tsapps/meteor/app/lib/server/methods/setRealName.tsapps/meteor/server/methods/hideRoom.tsapps/meteor/server/methods/ignoreUser.tsapps/meteor/server/methods/deleteUser.tsapps/meteor/app/lib/server/methods/saveSetting.tsapps/meteor/app/lib/server/methods/deleteUserOwnAccount.tsapps/meteor/app/importer/server/methods/getLatestImportOperations.tsapps/meteor/app/lib/server/methods/setEmail.tsapps/meteor/app/discussion/server/methods/createDiscussion.ts
📚 Learning: 2026-03-09T18:39:14.020Z
Learnt from: Harxhit
Repo: RocketChat/Rocket.Chat PR: 39476
File: apps/meteor/server/methods/addAllUserToRoom.ts:0-0
Timestamp: 2026-03-09T18:39:14.020Z
Learning: When implementing batch processing in server methods, favor a single data pass to collect both the items and any derived fields needed for validation. Use the same dataset for both validation and processing to avoid races between validation and execution, and document the approach in code comments. Apply this pattern to similar Meteor Rocket.Chat server methods under apps/meteor/server/methods to prevent race conditions and ensure consistent batch behavior.
Applied to files:
apps/meteor/server/methods/resetAvatar.tsapps/meteor/server/methods/toggleFavorite.tsapps/meteor/server/methods/getRoomNameById.tsapps/meteor/server/methods/removeUserFromRoom.tsapps/meteor/server/methods/channelsList.tsapps/meteor/server/methods/messageSearch.tsapps/meteor/server/methods/hideRoom.tsapps/meteor/server/methods/ignoreUser.tsapps/meteor/server/methods/deleteUser.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/server/methods/resetAvatar.tsapps/meteor/server/methods/toggleFavorite.tsapps/meteor/app/e2e/server/methods/fetchMyKeys.tsapps/meteor/app/importer/server/methods/startImport.tsapps/meteor/app/threads/server/methods/followMessage.tsapps/meteor/server/methods/getRoomNameById.tsapps/meteor/server/methods/removeUserFromRoom.tsapps/meteor/app/importer/server/methods/getImportProgress.tsapps/meteor/app/message-pin/server/pinMessage.tsapps/meteor/app/e2e/server/methods/setUserPublicAndPrivateKeys.tsapps/meteor/app/lib/server/methods/archiveRoom.tsapps/meteor/app/lib/server/methods/saveSettings.tsapps/meteor/server/methods/channelsList.tsapps/meteor/app/mentions/server/methods/getUserMentionsByChannel.tsapps/meteor/app/message-mark-as-unread/server/unreadMessages.tsapps/meteor/app/importer/server/methods/uploadImportFile.tsapps/meteor/app/importer/server/methods/downloadPublicImportFile.tsapps/meteor/app/lib/server/methods/unarchiveRoom.tsapps/meteor/app/statistics/server/methods/getStatistics.tsapps/meteor/app/lib/server/methods/createChannel.tsapps/meteor/app/threads/server/methods/unfollowMessage.tsapps/meteor/app/importer/server/methods/getImportFileData.tsapps/meteor/app/lib/server/methods/getChannelHistory.tsapps/meteor/app/threads/server/methods/getThreadsList.tsapps/meteor/server/methods/messageSearch.tsapps/meteor/app/lib/server/methods/setRealName.tsapps/meteor/server/methods/hideRoom.tsapps/meteor/server/methods/ignoreUser.tsapps/meteor/server/methods/deleteUser.tsapps/meteor/app/lib/server/methods/saveSetting.tsapps/meteor/app/lib/server/methods/deleteUserOwnAccount.tsapps/meteor/app/importer/server/methods/getLatestImportOperations.tsapps/meteor/app/lib/server/methods/setEmail.tsapps/meteor/app/discussion/server/methods/createDiscussion.ts
🧬 Code graph analysis (3)
apps/meteor/app/lib/server/methods/saveSetting.ts (1)
apps/meteor/app/lib/server/lib/notifyListener.ts (1)
notifyOnSettingChanged(352-357)
scripts/add-ddp-deprecation.mjs (1)
scripts/audit-ddp-methods.mjs (1)
main(443-528)
scripts/audit-ddp-methods.mjs (1)
scripts/add-ddp-deprecation.mjs (1)
main(216-302)
🪛 LanguageTool
docs/ddp-replacement-review.md
[style] ~8-~8: Did you mean ‘different from’? ‘Different than’ is often considered colloquial style.
Context: ...int does something materially different than the DDP method; the mapping is misleadi...
(DIFFERENT_THAN)
[style] ~63-~63: Possibly, ‘apparently’ is redundant. Consider using “only”.
Context: ... DDP path uses / separator; the regex apparently only matched dotted method names. | | `getRe...
(ADVERB_ONLY)
🔇 Additional comments (31)
apps/meteor/app/discussion/server/methods/createDiscussion.ts (1)
18-18: LGTM!Also applies to: 89-89, 251-251
apps/meteor/app/e2e/server/methods/fetchMyKeys.ts (1)
5-5: LGTM!Also applies to: 16-16
apps/meteor/app/e2e/server/methods/setUserPublicAndPrivateKeys.ts (1)
5-5: LGTM!Also applies to: 46-46
apps/meteor/app/importer/server/methods/downloadPublicImportFile.ts (1)
12-12: LGTM!Also applies to: 88-88
apps/meteor/app/importer/server/methods/getImportFileData.ts (1)
11-11: LGTM!Also applies to: 75-75
apps/meteor/app/importer/server/methods/getImportProgress.ts (1)
8-8: LGTM!Also applies to: 36-36
apps/meteor/app/importer/server/methods/getLatestImportOperations.ts (1)
7-7: LGTM!Also applies to: 30-30
apps/meteor/app/importer/server/methods/startImport.ts (1)
9-9: LGTM!Also applies to: 37-37
apps/meteor/app/importer/server/methods/uploadImportFile.ts (1)
9-9: LGTM!Also applies to: 68-68
apps/meteor/app/lib/server/methods/archiveRoom.ts (1)
11-11: LGTM!Also applies to: 46-46
apps/meteor/app/lib/server/methods/createChannel.ts (1)
9-9: LGTM!Also applies to: 64-64
apps/meteor/app/lib/server/methods/deleteUserOwnAccount.ts (1)
12-12: LGTM!Also applies to: 67-67
apps/meteor/app/lib/server/methods/getChannelHistory.ts (1)
10-10: LGTM!Also applies to: 159-159
apps/meteor/app/lib/server/methods/saveSetting.ts (1)
12-12: LGTM!Also applies to: 24-24, 79-81
apps/meteor/app/lib/server/methods/setEmail.ts (1)
9-9: LGTM!Also applies to: 28-28, 43-43
apps/meteor/app/lib/server/methods/setRealName.ts (1)
8-8: LGTM!Also applies to: 19-19
apps/meteor/app/lib/server/methods/unarchiveRoom.ts (1)
9-9: LGTM!Also applies to: 41-41
apps/meteor/app/mentions/server/methods/getUserMentionsByChannel.ts (1)
8-8: LGTM!Also applies to: 42-42
apps/meteor/app/message-mark-as-unread/server/unreadMessages.ts (1)
7-7: LGTM!Also applies to: 85-85
apps/meteor/app/message-pin/server/pinMessage.ts (1)
14-14: LGTM!Also applies to: 202-202, 215-215
apps/meteor/app/statistics/server/methods/getStatistics.ts (1)
5-5: LGTM!Also applies to: 17-17
apps/meteor/app/threads/server/methods/followMessage.ts (1)
10-10: LGTM!Also applies to: 54-54
apps/meteor/app/threads/server/methods/getThreadsList.ts (1)
7-7: LGTM!Also applies to: 21-21
apps/meteor/app/threads/server/methods/unfollowMessage.ts (1)
10-10: LGTM!Also applies to: 54-54
apps/meteor/server/methods/deleteUser.ts (1)
9-9: LGTM!Also applies to: 50-50
apps/meteor/server/methods/getRoomNameById.ts (1)
8-8: LGTM!Also applies to: 19-19
apps/meteor/server/methods/ignoreUser.ts (1)
6-6: LGTM!Also applies to: 48-48
apps/meteor/server/methods/messageSearch.ts (1)
8-8: LGTM!Also applies to: 91-91
apps/meteor/server/methods/channelsList.ts (1)
24-24: ⚡ Quick winDeprecation mapping should include the private-room REST replacement for
channelsList.
apps/meteor/server/methods/channelsList.tsline 24 mapschannelsListonly to/v1/channels.list, but the method implementation returns private rooms whenchannelType !== 'public'(guarded byview-p-room). Check whether/v1/channels.listcovers private rooms; if not, add the corresponding private-room REST endpoint to the deprecation mapping.apps/meteor/server/methods/resetAvatar.ts (1)
9-9: LGTM!Also applies to: 53-53
apps/meteor/server/methods/toggleFavorite.ts (1)
7-7: LGTM!Also applies to: 34-34
Comment on lines
+133
to
+134
| ip: this.connection.clientAddress || '', | ||
| useragent: this.connection.httpHeaders['user-agent'] || '', |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Direct access patterns in Meteor methods =="
rg -n -C2 "this\.connection\.(clientAddress|httpHeaders)" --type ts
echo
echo "== Optional-chained access patterns in Meteor methods =="
rg -n -C2 "this\.connection\?\.(clientAddress|httpHeaders)" --type ts
echo
echo "== saveSettings method context =="
rg -n -C4 "saveSettings: twoFactorRequired\(async function" --type tsRepository: RocketChat/Rocket.Chat
Length of output: 2943
Guard this.connection in saveSettings when reading IP/user-agent.
apps/meteor/app/lib/server/methods/saveSettings.ts lines 133-134 read this.connection.clientAddress and this.connection.httpHeaders['user-agent'] without checks; other Meteor methods in the repo use optional chaining for the same fields, so this should also be guarded to prevent runtime failure during settings persistence.
✅ Proposed fix
- ip: this.connection.clientAddress || '',
- useragent: this.connection.httpHeaders['user-agent'] || '',
+ ip: this.connection?.clientAddress || '',
+ useragent: this.connection?.httpHeaders?.['user-agent'] || '',📝 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
| ip: this.connection.clientAddress || '', | |
| useragent: this.connection.httpHeaders['user-agent'] || '', | |
| ip: this.connection?.clientAddress || '', | |
| useragent: this.connection?.httpHeaders?.['user-agent'] || '', |
🤖 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/app/lib/server/methods/saveSettings.ts` around lines 133 - 134,
In saveSettings, guard access to this.connection before reading clientAddress
and httpHeaders: replace direct reads of this.connection.clientAddress and
this.connection.httpHeaders['user-agent'] with optional-chaining and safe
fallback (e.g. this.connection?.clientAddress ?? '' and
this.connection?.httpHeaders?.['user-agent'] ?? '') so IP and user-agent are
safely handled when connection is undefined.
Comment on lines
+26
to
+33
| only: (() => { | ||
| const i = argv.indexOf('--only'); | ||
| return i >= 0 ? argv[i + 1] : 'all'; | ||
| })(), | ||
| limit: (() => { | ||
| const i = argv.indexOf('--limit'); | ||
| return i >= 0 ? Number(argv[i + 1]) : Infinity; | ||
| })(), |
Contributor
There was a problem hiding this comment.
Validate --only and --limit to prevent silent no-op runs.
At Lines 26–33, invalid --only values or non-numeric --limit currently fail silently and can generate an empty plan without signaling misuse.
Patch
const flags = {
apply: argv.includes('--apply'),
dryRun: argv.includes('--dry-run') || !argv.includes('--apply'),
only: (() => {
const i = argv.indexOf('--only');
return i >= 0 ? argv[i + 1] : 'all';
})(),
limit: (() => {
const i = argv.indexOf('--limit');
return i >= 0 ? Number(argv[i + 1]) : Infinity;
})(),
};
+
+if (!['all', 'orphans', 'used-with-rest'].includes(flags.only)) {
+ throw new Error(`Invalid --only value: ${flags.only}`);
+}
+if (!Number.isFinite(flags.limit) && flags.limit !== Infinity) {
+ throw new Error(`Invalid --limit value: ${flags.limit}`);
+}Also applies to: 199-214
🤖 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 `@scripts/add-ddp-deprecation.mjs` around lines 26 - 33, Validate the parsed
argv values for the IIFE-produced only and limit variables: for the only
assignment (where argv is checked for '--only'), ensure the extracted value is
one of the permitted plan names (compare against your canonical list, e.g.
PLAN_NAMES or allowedPlans) and if not print a clear error and exit non-zero;
for the limit assignment (where argv is checked for '--limit'), ensure
Number(argv[i+1]) yields a finite positive integer (use Number.isFinite and > 0
or Number.isInteger as appropriate) and if not print a clear error and exit
non-zero; apply the same validation logic to the duplicate parsing block later
in the file (the similar code around lines 199–214) so invalid inputs do not
produce a silent no-op.
Comment on lines
+181
to
+244
| // Skip the entire value of this entry so wrapper-call names like | ||
| // `twoFactorRequired(async function...)` aren't treated as keys. | ||
| let q = p; | ||
| while (q < body.length) { | ||
| const cq = body[q]; | ||
| if (cq === '/' && body[q + 1] === '/') { | ||
| const nl = body.indexOf('\n', q); | ||
| q = nl === -1 ? body.length : nl; | ||
| continue; | ||
| } | ||
| if (cq === '/' && body[q + 1] === '*') { | ||
| const end = body.indexOf('*/', q + 2); | ||
| q = end === -1 ? body.length : end + 2; | ||
| continue; | ||
| } | ||
| if (cq === '"' || cq === "'" || cq === '`') { | ||
| const qq = cq; | ||
| q++; | ||
| while (q < body.length) { | ||
| if (body[q] === '\\') { q += 2; continue; } | ||
| if (body[q] === qq) { q++; break; } | ||
| if (qq === '`' && body[q] === '$' && body[q + 1] === '{') { | ||
| q += 2; | ||
| let td = 1; | ||
| while (q < body.length && td > 0) { | ||
| if (body[q] === '{') td++; | ||
| else if (body[q] === '}') td--; | ||
| q++; | ||
| } | ||
| continue; | ||
| } | ||
| q++; | ||
| } | ||
| continue; | ||
| } | ||
| if (cq === '(' || cq === '{' || cq === '[') { | ||
| const open = cq; | ||
| const close = cq === '(' ? ')' : cq === '{' ? '}' : ']'; | ||
| let pd = 1; | ||
| q++; | ||
| while (q < body.length && pd > 0) { | ||
| const cc2 = body[q]; | ||
| if (cc2 === '"' || cc2 === "'" || cc2 === '`') { | ||
| const qq = cc2; | ||
| q++; | ||
| while (q < body.length) { | ||
| if (body[q] === '\\') { q += 2; continue; } | ||
| if (body[q] === qq) { q++; break; } | ||
| q++; | ||
| } | ||
| continue; | ||
| } | ||
| if (cc2 === open) pd++; | ||
| else if (cc2 === close) pd--; | ||
| q++; | ||
| } | ||
| continue; | ||
| } | ||
| if (cq === ',') { q++; break; } | ||
| if (cq === '}') break; // end of methods block | ||
| q++; | ||
| } | ||
| i = q; | ||
| continue; |
Contributor
There was a problem hiding this comment.
Top-level key extraction is producing false DDP method names.
The value-skipping scanner in this block is leaking nested identifiers as top-level keys, which causes bogus methods to be audited/deprecated (downstream evidence: docs/ddp-deprecation-plan.json includes context and settings entries under apps/meteor/app/search/server/methods.ts). This is a correctness issue in the parser logic.
Suggested direction
- // ad-hoc skip logic that tracks one delimiter kind at a time
+ // switch to a unified token scanner with a delimiter stack for (), {}, []
+ // while skipping comments/strings/templates, and only emit keys when stack depth is 0.
+ // then skip each entry value until the next top-level comma or block end.🤖 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 `@scripts/audit-ddp-methods.mjs` around lines 181 - 244, The value-skipping
loop (variables body, p, q and the loop that ends with i = q) is treating nested
object keys as top-level keys; fix it by introducing a top-level depth counter
and only treat ',' or '}' as terminators when that top-level depth is zero—i.e.,
increment/decrement depth on encountering '{' or '}' (and also handle '(' and
'[' as you already do for nested groups), and continue skipping everything
(strings, comments, template expressions) until the top-level depth returns to
zero; update the loop that currently breaks on ',' or '}' so it checks that
depth === 0 before breaking and then set i = q as before.
sampaiodiego
requested changes
May 25, 2026
Member
sampaiodiego
left a comment
sampaiodiego
left a comment
There was a problem hiding this comment.
I didn't check all the scripts and docs, are they required?
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async setEmail(email) { | ||
| methodDeprecationLogger.method('setEmail', '9.0.0', '/v1/users.update'); |
Member
There was a problem hiding this comment.
Suggested change
| methodDeprecationLogger.method('setEmail', '9.0.0', '/v1/users.update'); | |
| methodDeprecationLogger.method('setEmail', '9.0.0', '/v1/users.updateOwnBasicInfo'); |
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async setRealName(name) { | ||
| methodDeprecationLogger.method('setRealName', '9.0.0', '/v1/users.update'); |
Member
There was a problem hiding this comment.
Suggested change
| methodDeprecationLogger.method('setRealName', '9.0.0', '/v1/users.update'); | |
| methodDeprecationLogger.method('setRealName', '9.0.0', '/v1/users.updateOwnBasicInfo'); |
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async hideRoom(rid) { | ||
| methodDeprecationLogger.method('hideRoom', '9.0.0', '/v1/rooms.leave'); |
Member
There was a problem hiding this comment.
Suggested change
| methodDeprecationLogger.method('hideRoom', '9.0.0', '/v1/rooms.leave'); | |
| methodDeprecationLogger.method('hideRoom', '9.0.0', '/v1/rooms.hide'); |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
4 issues found across 45 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![hacktron-app[bot]](https://avatars.githubusercontent.com/in/1250513?s=60&v=4){kind=small}
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async downloadPublicImportFile(fileUrl: string, importerKey: string) { | ||
| methodDeprecationLogger.method('downloadPublicImportFile', '9.0.0', '/v1/downloadPublicImportFile'); |
There was a problem hiding this comment.
🔴 High: Server-Side Request Forgery (SSRF) in Import File Download
The downloadPublicImportFile method allows a user with the run-import permission to specify an arbitrary URL for the server to fetch. The downloadHttpFile function uses this user-supplied URL to perform an outbound network request using http.get or https.get without any validation or restriction on the target destination. This allows an attacker to perform Server-Side Request Forgery (SSRF), enabling them to scan internal networks, interact with internal services, or access cloud metadata services.
Steps to Reproduce
- Authenticate as a user with the 'run-import' permission.
- Send a request to the
downloadPublicImportFilemethod with a target URL pointing to an internal resource (e.g., http://169.254.169.254/ or http://localhost:8080/). - The server will attempt to fetch the URL, confirming the SSRF vulnerability.
# Assuming the user has the 'run-import' permission, the following request triggers an SSRF.
# Replace <URL> with an internal target (e.g., http://169.254.169.254/latest/meta-data/)
curl -X POST http://localhost:3000/api/v1/method.call/downloadPublicImportFile \
-H 'Content-Type: application/json' \
-d '{"message":"{\"msg\":\"method\",\"method\":\"downloadPublicImportFile\",\"params\":[\"http://169.254.169.254/\",\"some-importer-key\"],\"id\":\"1\"}"}'Trace
graph TD
subgraph SG0 ["./Rocket.Chat/apps/meteor/app/authorization/server/functions/hasPermission.ts"]
hasPermissionAsync["Checks if a user has a specific permission, optionally scoped to a room."]
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG1 ["./Rocket.Chat/apps/meteor/app/importer/server/methods/downloadPublicImportFile.ts"]
downloadHttpFile["downloadHttpFile"]
copyLocalFile["copyLocalFile"]
executeDownloadPublicImportFile["executeDownloadPublicImportFile"]
downloadPublicImportFile{{"Downloads a public import file after checking user permissions."}}
end
style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG2 ["./Rocket.Chat/apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts"]
compareVersions["Checks if a version is below the threshold and throws an error if configured."]
method["Logs or throws errors for deprecated method usage."]
warn["Logs a warning message for deprecations."]
end
style SG2 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG3 ["./Rocket.Chat/apps/meteor/client/meteor/overrides/userAndUsers.ts"]
Meteor.userId["Overrides Meteor.userId to return the current user ID from the reactive Zustand store."]
end
style SG3 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG4 ["./Rocket.Chat/apps/meteor/client/meteor/user.ts"]
watchUserId["Watches the current user ID from the reactive store."]
end
style SG4 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG5 ["./Rocket.Chat/apps/meteor/client/meteor/watch.ts"]
watch["watch"]
end
style SG5 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG6 ["./Rocket.Chat/apps/meteor/server/services/authorization/service.ts"]
Authorization.hasPermission["Checks if a user has a specific permission."]
Authorization.all["Authorization.all"]
end
style SG6 fill:#2a2a2a,stroke:#444,color:#aaa
downloadPublicImportFile --> Meteor.userId
downloadPublicImportFile --> hasPermissionAsync
downloadPublicImportFile --> method
downloadPublicImportFile --> executeDownloadPublicImportFile
Meteor.userId --> watchUserId
hasPermissionAsync --> Authorization.hasPermission
method --> compareVersions
method --> warn
executeDownloadPublicImportFile --> downloadHttpFile
executeDownloadPublicImportFile --> copyLocalFile
watchUserId --> watch
Authorization.hasPermission --> Authorization.all
warn --> compareVersions
warn --> warn
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/app/importer/server/methods/downloadPublicImportFile.ts
Lines: 88
Severity: high
Vulnerability: Server-Side Request Forgery (SSRF) in Import File Download
Description:
The `downloadPublicImportFile` method allows a user with the `run-import` permission to specify an arbitrary URL for the server to fetch. The `downloadHttpFile` function uses this user-supplied URL to perform an outbound network request using `http.get` or `https.get` without any validation or restriction on the target destination. This allows an attacker to perform Server-Side Request Forgery (SSRF), enabling them to scan internal networks, interact with internal services, or access cloud metadata services.
Proof of Concept:
**Steps to Reproduce**
1. Authenticate as a user with the 'run-import' permission.
2. Send a request to the `downloadPublicImportFile` method with a target URL pointing to an internal resource (e.g., http://169.254.169.254/ or http://localhost:8080/).
3. The server will attempt to fetch the URL, confirming the SSRF vulnerability.
```bash
# Assuming the user has the 'run-import' permission, the following request triggers an SSRF.
# Replace <URL> with an internal target (e.g., http://169.254.169.254/latest/meta-data/)
curl -X POST http://localhost:3000/api/v1/method.call/downloadPublicImportFile \
-H 'Content-Type: application/json' \
-d '{"message":"{\"msg\":\"method\",\"method\":\"downloadPublicImportFile\",\"params\":[\"http://169.254.169.254/\",\"some-importer-key\"],\"id\":\"1\"}"}'
```
Affected Code:
function downloadHttpFile(fileUrl: string, writeStream: fs.WriteStream): void {
const protocol = fileUrl.startsWith('https') ? https : http;
protocol.get(fileUrl, (response) => {
response.pipe(writeStream);
});
}
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.
- hideRoom now points to /v1/rooms.hide (was /v1/rooms.leave) — DDP method calls Subscriptions.hideByRoomIdAndUserId, not removeByRoomIdAndUserId - removeUserFromRoom now lists both /v1/channels.kick and /v1/groups.kick since the DDP method does not branch on room.t at log time
Member
Author
Bot review summaryPushed fix
@sampaiodiego on scripts/docsThe four
Happy to move them under Not fixed in this PR (pre-existing, separate issue)
Script-scanner nits (cubic P1 + coderabbit on
|
…icInfo Both methods are self-service (current user updates own info), so the proper REST replacement is /v1/users.updateOwnBasicInfo, not /v1/users.update (which is admin-only).
Maintainer asked whether the planning artifacts were required for this PR — they aren't. The deprecation-log changes in this PR stand on their own; the audit/inject/strip scripts live in the related cleanup branches and can be revived from history if we need to re-run the sweep.
… replacement Audit cross-check against /v1/ routes found 33 more orphan methods that already have an equivalent REST endpoint. Adding the deprecation log ensures any runtime caller (server-side, SDKs, mobile) sees the warning before the method is removed in 9.0.0. Moved from #40657 (originally classified 'no REST replacement') after re-validating each entry against packages/rest-typings and apps/meteor/app/api/server/v1.
ggazzo
added a commit
that referenced
this pull request
May 26, 2026
Audit cross-check against existing /v1/ routes showed these methods are not actually 'no replacement' — they have working REST equivalents. Their deprecation log calls moved to #40654 (orphan + REST replacement). This PR keeps only methods that genuinely lack a REST counterpart.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async updateIncomingIntegration(integrationId, integration) { | ||
| methodDeprecationLogger.method('updateIncomingIntegration', '9.0.0', '/v1/integrations.update'); |
There was a problem hiding this comment.
🔴 High: Privilege Escalation via Incoming Integration Update
The updateIncomingIntegration method allows a user with permission to manage their own integrations to update the 'username' associated with an incoming integration. The method then calls addUserRolesAsync(user._id, ['bot']) using the ID of the user associated with the updated username. An attacker can set the integration's username to an arbitrary existing user (e.g., an administrator), and the system will automatically grant the 'bot' role to that user. While the 'bot' role is restricted, this is an unauthorized elevation of privilege for the targeted account.
Steps to Reproduce
- Log in as a user with
manage-own-incoming-integrationspermission. - Create or find an existing incoming integration owned by the user.
- Call the
updateIncomingIntegrationmethod, providing the integration ID and ausernamefield set to the username of an administrator or any other target user. - The system updates the integration and grants the 'bot' role to the target user.
# Conceptual Python script using DDP to call the method
from meteor_ddp import DDPClient # Hypothetical library
client = DDPClient('ws://localhost:3000/websocket')
client.login('user', 'password')
client.call('updateIncomingIntegration', ['<integration-id>', {'username': '<admin-username>'}])# This is a conceptual PoC.
# 1. Authenticate as a standard user who has 'manage-own-incoming-integrations' permission.
# 2. Identify an existing integration ID owned by the user.
# 3. Call `updateIncomingIntegration` with the integration ID and set `username` to an admin's username.
# 4. The admin user will now have the 'bot' role added to their account.# Conceptual curl command to call the Meteor method
curl -X POST http://localhost:3000/api/v1/method.call/updateIncomingIntegration \
-H "X-Auth-Token: <user-token>" \
-H "X-User-Id: <user-id>" \
-H "Content-Type: application/json" \
-d '{"message": "{\"msg\":\"method\",\"method\":\"updateIncomingIntegration\",\"params\":[\"<integration-id>\",{\"username\":\"<admin-username>\"}]}"}'PoC Url: /api/v1/method.call/updateIncomingIntegration
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/app/integrations/server/methods/incoming/updateIncomingIntegration.ts
Lines: 195
Severity: high
Vulnerability: Privilege Escalation via Incoming Integration Update
Description:
The `updateIncomingIntegration` method allows a user with permission to manage their own integrations to update the 'username' associated with an incoming integration. The method then calls `addUserRolesAsync(user._id, ['bot'])` using the ID of the user associated with the updated username. An attacker can set the integration's username to an arbitrary existing user (e.g., an administrator), and the system will automatically grant the 'bot' role to that user. While the 'bot' role is restricted, this is an unauthorized elevation of privilege for the targeted account.
Proof of Concept:
**Steps to Reproduce**
1. Log in as a user with `manage-own-incoming-integrations` permission.
2. Create or find an existing incoming integration owned by the user.
3. Call the `updateIncomingIntegration` method, providing the integration ID and a `username` field set to the username of an administrator or any other target user.
4. The system updates the integration and grants the 'bot' role to the target user.
```python
# Conceptual Python script using DDP to call the method
from meteor_ddp import DDPClient # Hypothetical library
client = DDPClient('ws://localhost:3000/websocket')
client.login('user', 'password')
client.call('updateIncomingIntegration', ['<integration-id>', {'username': '<admin-username>'}])
```
```bash
# This is a conceptual PoC.
# 1. Authenticate as a standard user who has 'manage-own-incoming-integrations' permission.
# 2. Identify an existing integration ID owned by the user.
# 3. Call `updateIncomingIntegration` with the integration ID and set `username` to an admin's username.
# 4. The admin user will now have the 'bot' role added to their account.
```
```bash
# Conceptual curl command to call the Meteor method
curl -X POST http://localhost:3000/api/v1/method.call/updateIncomingIntegration \
-H "X-Auth-Token: <user-token>" \
-H "X-User-Id: <user-id>" \
-H "Content-Type: application/json" \
-d '{"message": "{\"msg\":\"method\",\"method\":\"updateIncomingIntegration\",\"params\":[\"<integration-id>\",{\"username\":\"<admin-username>\"}]}"}'
```
PoC Url: /api/v1/method.call/updateIncomingIntegration
Affected Code:
await addUserRolesAsync(user._id, ['bot']);
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.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async addRoomModerator(rid, userId) { | ||
| methodDeprecationLogger.method('addRoomModerator', '9.0.0', ['/v1/channels.addModerator', '/v1/groups.addModerator']); |
There was a problem hiding this comment.
🔴 High: Authorization Bypass in Federated Room Moderator Assignment
In addRoomModerator, the authorization check for adding a room moderator is bypassed if the room is federated. The condition if (!(await hasPermissionAsync(fromUserId, 'set-moderator', rid)) && !isFederated) evaluates to false when isFederated is true, regardless of whether the fromUserId has the set-moderator permission. This allows any user (including unprivileged ones) to add themselves or others as moderators in any federated room.
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/server/methods/addRoomModerator.ts
Lines: 114
Severity: high
Vulnerability: Authorization Bypass in Federated Room Moderator Assignment
Description:
In `addRoomModerator`, the authorization check for adding a room moderator is bypassed if the room is federated. The condition `if (!(await hasPermissionAsync(fromUserId, 'set-moderator', rid)) && !isFederated)` evaluates to false when `isFederated` is true, regardless of whether the `fromUserId` has the `set-moderator` permission. This allows any user (including unprivileged ones) to add themselves or others as moderators in any federated room.
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.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async removeRoomModerator(rid, userId) { | ||
| methodDeprecationLogger.method('removeRoomModerator', '9.0.0', ['/v1/channels.removeModerator', '/v1/groups.removeModerator']); |
There was a problem hiding this comment.
🔴 High: Authorization Bypass in Federated Room Moderator Removal
The removeRoomModerator method performs an authorization check for the 'set-moderator' permission, but it explicitly bypasses this check if the room is federated (!isRoomFederated(room)). This allows any authenticated user to remove a moderator from a federated room without possessing the required administrative permissions.
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/server/methods/removeRoomModerator.ts
Lines: 105
Severity: high
Vulnerability: Authorization Bypass in Federated Room Moderator Removal
Description:
The `removeRoomModerator` method performs an authorization check for the 'set-moderator' permission, but it explicitly bypasses this check if the room is federated (`!isRoomFederated(room)`). This allows any authenticated user to remove a moderator from a federated room without possessing the required administrative permissions.
Affected Code:
if (!(await hasPermissionAsync(fromUserId, 'set-moderator', rid)) && !isRoomFederated(room)) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', {
method: 'removeRoomModerator',
});
}
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.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async removeRoomOwner(rid, userId) { | ||
| methodDeprecationLogger.method('removeRoomOwner', '9.0.0', ['/v1/channels.removeOwner', '/v1/groups.removeOwner']); |
There was a problem hiding this comment.
🔴 High: Authorization Bypass in Federated Room Owner Removal
The removeRoomOwner method performs an authorization check for the 'set-owner' permission, but it explicitly bypasses this check if the room is federated (!isRoomFederated(room)). This allows any authenticated user to remove an owner from a federated room without possessing the required administrative permissions.
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/server/methods/removeRoomOwner.ts
Lines: 110
Severity: high
Vulnerability: Authorization Bypass in Federated Room Owner Removal
Description:
The `removeRoomOwner` method performs an authorization check for the 'set-owner' permission, but it explicitly bypasses this check if the room is federated (`!isRoomFederated(room)`). This allows any authenticated user to remove an owner from a federated room without possessing the required administrative permissions.
Affected Code:
if (!(await hasPermissionAsync(fromUserId, 'set-owner', rid)) && !isRoomFederated(room)) {
throw new Meteor.Error('error-not-allowed', 'Not allowed', {
method: 'removeRoomOwner',
});
}
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.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async addOutgoingIntegration(integration: INewOutgoingIntegration): Promise<IOutgoingIntegration> { | ||
| methodDeprecationLogger.method('addOutgoingIntegration', '9.0.0', '/v1/integrations.create'); |
There was a problem hiding this comment.
🔴 High: Insufficient Authorization Check in Outgoing Integration Creation (Impersonation and Scope Bypass)
The addOutgoingIntegration function checks if the user has manage-outgoing-integrations or manage-own-outgoing-integrations permissions. However, it fails to verify if the user is authorized to perform the action in the specific context (e.g., creating an integration that targets a specific room or uses a specific user identity). An attacker with manage-own-outgoing-integrations could potentially create integrations that impersonate other users or interact with rooms they should not have access to, as the function does not validate the integration's scope against the user's permissions.
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/app/integrations/server/methods/outgoing/addOutgoingIntegration.ts
Lines: 79
Severity: high
Vulnerability: Insufficient Authorization Check in Outgoing Integration Creation (Impersonation and Scope Bypass)
Description:
The `addOutgoingIntegration` function checks if the user has `manage-outgoing-integrations` or `manage-own-outgoing-integrations` permissions. However, it fails to verify if the user is authorized to perform the action in the specific context (e.g., creating an integration that targets a specific room or uses a specific user identity). An attacker with `manage-own-outgoing-integrations` could potentially create integrations that impersonate other users or interact with rooms they should not have access to, as the function does not validate the integration's scope against the user's permissions.
Affected Code:
if (
!userId ||
(!(await hasPermissionAsync(userId, 'manage-outgoing-integrations')) &&
!(await hasPermissionAsync(userId, 'manage-own-outgoing-integrations')))
) {
throw new Meteor.Error('not_authorized');
}
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.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async addIncomingIntegration(integration: INewIncomingIntegration): Promise<IIncomingIntegration> { | ||
| methodDeprecationLogger.method('addIncomingIntegration', '9.0.0', '/v1/integrations.create'); |
There was a problem hiding this comment.
🔴 High: Privilege Escalation via Incoming Integration Creation
The addIncomingIntegration method allows users with the 'manage-own-incoming-integrations' permission to create an incoming integration. As part of this process, the system automatically assigns the 'bot' role to the user account specified in the integration ('integration.username'). Because an attacker with the 'manage-own-incoming-integrations' permission can create an integration for any existing user (including their own account), they can effectively elevate the privileges of that user to the 'bot' role. The 'bot' role is considered a non-attacker-assumable role with elevated capabilities like message impersonation, leading to unauthorized privilege escalation.
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/app/integrations/server/methods/incoming/addIncomingIntegration.ts
Lines: 179
Severity: high
Vulnerability: Privilege Escalation via Incoming Integration Creation
Description:
The `addIncomingIntegration` method allows users with the 'manage-own-incoming-integrations' permission to create an incoming integration. As part of this process, the system automatically assigns the 'bot' role to the user account specified in the integration ('integration.username'). Because an attacker with the 'manage-own-incoming-integrations' permission can create an integration for any existing user (including their own account), they can effectively elevate the privileges of that user to the 'bot' role. The 'bot' role is considered a non-attacker-assumable role with elevated capabilities like message impersonation, leading to unauthorized privilege escalation.
Affected Code:
await addUserRolesAsync(user._id, ['bot']);
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.
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async 'raix:push-update'(options) { | ||
| methodDeprecationLogger.method('raix:push-update', '9.0.0', '/v1/push.token'); |
There was a problem hiding this comment.
🟡 Medium: Insecure Direct Object Reference (IDOR) in Push Token Update allows unauthorized token modification
The 'raix:push-update' Meteor method allows a user to update a push token by optionally providing an 'id' field. The provided 'id' is not validated to ensure it belongs to the authenticated user. An attacker can provide an 'id' belonging to another user's push token document, and the server will update that document with the attacker's provided data, including associating it with the attacker's 'userId'. This leads to unauthorized modification of other users' push notification settings.
Steps to Reproduce
- Authenticate as an attacker.
- Identify or guess a victim's push token ID.
- Call the 'raix:push-update' method via DDP, providing the victim's push token ID in the 'id' field, along with a new token and the attacker's 'userId'.
- The server will update the victim's push token document, associating it with the attacker's 'userId'.
Fix with AI
Fix the following security vulnerability found by Hacktron.
File: apps/meteor/app/push/server/methods.ts
Lines: 31
Severity: medium
Vulnerability: Insecure Direct Object Reference (IDOR) in Push Token Update allows unauthorized token modification
Description:
The 'raix:push-update' Meteor method allows a user to update a push token by optionally providing an 'id' field. The provided 'id' is not validated to ensure it belongs to the authenticated user. An attacker can provide an 'id' belonging to another user's push token document, and the server will update that document with the attacker's provided data, including associating it with the attacker's 'userId'. This leads to unauthorized modification of other users' push notification settings.
Proof of Concept:
**Steps to Reproduce**
1. Authenticate as an attacker.
2. Identify or guess a victim's push token ID.
3. Call the 'raix:push-update' method via DDP, providing the victim's push token ID in the 'id' field, along with a new token and the attacker's 'userId'.
4. The server will update the victim's push token document, associating it with the attacker's 'userId'.
Affected Code:
return Push.registerPushToken({
...(options.id && { _id: options.id }),
token: options.token,
appName: options.appName,
authToken,
userId: this.userId,
...(options.metadata && { metadata: options.metadata }),
});
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.
…ement - license:getTags → /v1/licenses.info (returns tags via License.getInfo) - UserPresence:setDefaultStatus → /v1/users.setStatus - getRoomJoinCode → /v1/channels.info, /v1/groups.info (joinCode field)
Contributor
There was a problem hiding this comment.
1 issue found across 33 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/emoji-custom/server/methods/uploadEmojiCustom.ts">
<violation number="1" location="apps/meteor/app/emoji-custom/server/methods/uploadEmojiCustom.ts:24">
P1: Incorrect REST path: `/v1/emoji-custom.create` should be `/api/v1/emoji-custom.create`. The Rocket.Chat REST API routes are mounted under `/api/v1/`, so the path in the deprecation logger will direct users to a 404.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async uploadEmojiCustom(binaryContent, contentType, emojiData) { | ||
| methodDeprecationLogger.method('uploadEmojiCustom', '9.0.0', '/v1/emoji-custom.create'); |
Contributor
There was a problem hiding this comment.
P1: Incorrect REST path: /v1/emoji-custom.create should be /api/v1/emoji-custom.create. The Rocket.Chat REST API routes are mounted under /api/v1/, so the path in the deprecation logger will direct users to a 404.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/emoji-custom/server/methods/uploadEmojiCustom.ts, line 24:
<comment>Incorrect REST path: `/v1/emoji-custom.create` should be `/api/v1/emoji-custom.create`. The Rocket.Chat REST API routes are mounted under `/api/v1/`, so the path in the deprecation logger will direct users to a 404.</comment>
<file context>
@@ -20,6 +21,7 @@ declare module '@rocket.chat/ddp-client' {
Meteor.methods<ServerMethods>({
async uploadEmojiCustom(binaryContent, contentType, emojiData) {
+ methodDeprecationLogger.method('uploadEmojiCustom', '9.0.0', '/v1/emoji-custom.create');
await uploadEmojiCustom(this.userId, binaryContent, contentType, emojiData);
},
</file context>
ggazzo
added a commit
that referenced
this pull request
May 26, 2026
Moved to #40654 (orphan + REST replacement) after route cross-check: - license:getTags → /v1/licenses.info - UserPresence:setDefaultStatus → /v1/users.setStatus - getRoomJoinCode → /v1/channels.info, /v1/groups.info Dropped entirely (not orphan — used dynamically by client): - blockUser - unblockUser useMethod(isUserBlocked ? 'unblockUser' : 'blockUser') in useBlockUserAction.ts wouldn't be caught by static audit, so deprecating them would throw in TEST_MODE-equipped UI test runs.
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/emoji-custom/server/methods/uploadEmojiCustom.ts">
<violation number="1" location="apps/meteor/app/emoji-custom/server/methods/uploadEmojiCustom.ts:24">
P1: Incorrect REST path: `/v1/emoji-custom.create` should be `/api/v1/emoji-custom.create`. The Rocket.Chat REST API routes are mounted under `/api/v1/`, so the path in the deprecation logger will direct users to a 404.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
sampaiodiego
requested changes
May 26, 2026
eslint --fix on the 25 method files that had the deprecation logger import inserted out of alphabetical order or missing the empty line between import groups.
- getUserStatusText → /v1/users.presence (per @sampaiodiego — presence endpoint returns statusText; users.info is broader) - getRoomJoinCode reverted to no-REST replacement: /v1/channels.info and /v1/groups.info don't expose the joinCode field (per cubic)
Contributor
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/meteor/app/user-status/server/methods/getUserStatusText.ts">
<violation number="1" location="apps/meteor/app/user-status/server/methods/getUserStatusText.ts:16">
P2: The deprecation logger points `getUserStatusText` to a REST endpoint with different behavior (`users.presence` excludes offline users), so migration guidance can be incorrect.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| Meteor.methods<ServerMethods>({ | ||
| async getUserStatusText(userId) { | ||
| methodDeprecationLogger.method('getUserStatusText', '9.0.0', '/v1/users.presence'); |
Contributor
There was a problem hiding this comment.
P2: The deprecation logger points getUserStatusText to a REST endpoint with different behavior (users.presence excludes offline users), so migration guidance can be incorrect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/app/user-status/server/methods/getUserStatusText.ts, line 16:
<comment>The deprecation logger points `getUserStatusText` to a REST endpoint with different behavior (`users.presence` excludes offline users), so migration guidance can be incorrect.</comment>
<file context>
@@ -13,7 +13,7 @@ declare module '@rocket.chat/ddp-client' {
Meteor.methods<ServerMethods>({
async getUserStatusText(userId) {
- methodDeprecationLogger.method('getUserStatusText', '9.0.0', '/v1/users.info');
+ methodDeprecationLogger.method('getUserStatusText', '9.0.0', '/v1/users.presence');
const currentUserId = Meteor.userId();
if (!currentUserId) {
</file context>
Suggested change
| methodDeprecationLogger.method('getUserStatusText', '9.0.0', '/v1/users.presence'); | |
| methodDeprecationLogger.method('getUserStatusText', '9.0.0', '/v1/users.info'); |
sampaiodiego
approved these changes
May 26, 2026
ggazzo
added
the
stat: QA assured
dionisio-bot
Bot
added
the
stat: ready to merge
ggazzo
added a commit
that referenced
this pull request
May 26, 2026
Adds methodDeprecationLogger.method('NAME', '9.0.0', []) inside each
Meteor.methods body for orphan DDP methods that genuinely lack a
REST equivalent. Methods covered (26 total in 21 files):
cloud:* (5 OAuth/registration internals), crowd_*, addSamlService,
addWebdavAccountByToken, botRequest, checkFederationConfiguration,
getRoomJoinCode (joinCode field not exposed via REST), getS3FileUrl,
joinDefaultChannels, loadLocale, messages/get (publication, not
method), OAuth.retrieveCredential (iframe-login), OEmbedCacheCleanup,
openRoom, push_test, removeSlackBridgeChannelLinks, resetIrcConnection,
restart_server, rocketchatSearch.suggest, saveAudioNotificationValue,
sendSMTPTestEmail.
Companion to #40654 which covers orphan methods with a REST replacement.
Refs: ARCH-2159
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
2 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
Adds
methodDeprecationLogger.method(name, '9.0.0', '/v1/...')to the Meteor (DDP) methods that meet both conditions:apps/meteor/app/api/server/v1(orapps/meteor/ee/.../api/server) that can act as the replacement.Methods that are still used are intentionally not deprecated here — the deprecation logger throws under
TEST_MODE, so a deprecation on a live method breaks the test suite. Those move to the migration PR (#40659). Methods without a REST replacement go to the no-replacement PR (#40657). Orphans being removed outright in 9.0.0 are in #40656.Each call adds a
// @deprecatedJSDoc above the registration so editors strike through the method name.Scope
43 methods, 43 logger calls, 43 files touched. Pre-existing deprecations (
sendFileMessage,starMessage,insertOrUpdateSound,uploadCustomSound) untouched.How
Two scripts under
scripts/:audit-ddp-methods.mjswalks the monorepo to enumerateMeteor.methodsregistrations, callers (Meteor.call,sdk.call,useMethod,callWithErrorHandling, …), and REST routes, producingdocs/ddp-audit.{json,md}.add-ddp-deprecation.mjsconsumes that audit and inserts the logger call into each orphan-with-REST method, adding the import in the relative-imports group. Handles shorthand,name: function (...), arrow, andtwoFactorRequired(async function/arrow ...)registration shapes.Both scripts are reproducible — future audits don't need a manual sweep. The DDP→REST mapping lives in
candidateRoutes()and has aREST_DENYLISTfor endpoints whose semantics don't actually line up.Caveats
useMethod(value)in adminMethodActionInput.tsx, or the/v1/method.call/:methodREST proxy) look orphan but are still reachable. The known dynamic-dispatch methods are on a skip list to keep them alive.Test plan
Task: ARCH-2158
Summary by CodeRabbit
Deprecations
Code Improvements