chore: migrate audit DDP methods to /v1/audit.* REST endpoints (EE)#40736
chore: migrate audit DDP methods to /v1/audit.* REST endpoints (EE)#40736ggazzo wants to merge 3 commits into
Conversation
Added three new REST endpoints (EE-only, license: auditing) covering the audit flows that previously only existed as DDP methods: - GET /v1/audit.auditions (auditGetAuditions) - POST /v1/audit.messages (auditGetMessages) - POST /v1/audit.omnichannel.messages (auditGetOmnichannelMessages) Method bodies extracted into apps/meteor/ee/server/lib/audit/functions.ts and reused by both DDP entrypoints (now thin + deprecation-logged) and the new REST handlers. Each REST endpoint: - requires the same per-action permission (can-audit / can-audit-log). - is rate-limited at 10/60s matching the DDP DDPRateLimiter rules. - writes the same AuditLog entry the DDP path produced. - accepts dates as ISO strings on the wire (parsed server-side). Client AuditLogTable + useAuditMutation swapped from useMethod to useEndpoint; date params serialized via toISOString(). DDP methods stay registered with deprecation logs pointing at the new routes until 9.0.0. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 7ce2aaa The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAudit business logic is extracted from inline DDP method handlers into a new ChangesAudit DDP→REST Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #40736 +/- ##
===========================================
+ Coverage 70.13% 70.15% +0.01%
===========================================
Files 3368 3368
Lines 130022 130022
Branches 22582 22985 +403
===========================================
+ Hits 91191 91215 +24
+ Misses 35519 35492 -27
- Partials 3312 3315 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
- Add response schemas to /v1/audit.{auditions,messages,omnichannel.messages}
so TypedOptions infers queryParams/bodyParams correctly.
- mapMessageFromApi() each REST message in useAuditMutation so caller
AuditResult receives IMessage[] (Date) instead of Serialized<IMessage>[].
- Cast AuditLogEntry value through unknown→IAuditLog since the REST
response carries dates as strings while the consumer reads IAuditLog
(the component formats the Date itself, so the cast is just to satisfy
TS — same pattern other audit-adjacent code uses).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/views/audit/components/AuditLogTable.tsx`:
- Around line 27-33: The audit log query is still force-casting the REST
response into IAuditLog instead of converting it to the client shape. Update
AuditLogTable’s useEndpoint/queryFn path to deserialize the payload before
caching it, mapping REST fields like ts and date values into the IAuditLog
structure the table and AuditLogEntry expect, similar to how useAuditMutation
handles REST-to-client conversion.
In `@apps/meteor/client/views/audit/hooks/useAuditMutation.ts`:
- Around line 19-27: The `useAuditMutation` branch for `type === 'l'` is
ignoring the `AuditFields` filter values by hardcoding `visitor` and `agent` to
empty strings. Update the `getOmnichannelAuditMessages` call to forward the
actual `visitor` and `agent` values from the current audit filters, using the
existing `AuditFields` data in `useAuditMutation` so omnichannel searches stay
scoped correctly.
In `@apps/meteor/ee/server/lib/audit/functions.ts`:
- Around line 44-46: The shared audit helper in functions.ts is emitting a stale
DDP-only deprecation warning from logic now used by REST routes. Remove the
console.warn from the type === 'l' branch in the shared function and keep
deprecation logging only in the DDP wrapper path that calls this helper, so REST
calls do not report a misleading “method” warning or 4.0.0 removal target.
- Around line 175-177: The omnichannel audit path in auditGetMessagesMethod is
missing the same livechat room restriction filter used elsewhere, so apply
livechat.applyRoomRestrictions before the
LivechatRooms.findByVisitorIdAndAgentId lookup. Update the audit messages flow
in functions.ts so the visitor/agent room search is constrained consistently
with the standard audit path, using the existing auditGetMessagesMethod and
livechat.applyRoomRestrictions symbols as the insertion point.
- Around line 178-182: The `findByVisitorIdAndAgentId`/audit query path should
guard the no-room case before building the Mongo filter. In the `rids`/`query`
निर्माण, return or throw early when `rooms` is empty so `rid: { $in: undefined
}` is never created. Keep the fix localized around the `rids` assignment and the
`query: Filter<IMessage>` construction.
In `@apps/meteor/ee/server/lib/audit/methods.ts`:
- Around line 37-53: The audit DDP methods currently validate only the date
fields, so untyped payloads can still reach the shared audit query builders with
missing users, type, visitor, or agent data. Update the method boundaries in
auditGetOmnichannelMessages, auditGetMessages, and auditGetAuditions to validate
the full params object before calling auditGetOmnichannelMessagesMethod,
auditGetMessagesMethod, and auditGetAuditionsMethod. Keep the existing date
checks, but add validation for all required fields in each request shape so
invalid input is rejected early.
🪄 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: 19e5eb95-46be-4ddd-908c-daa3adeb7a77
📒 Files selected for processing (7)
.changeset/ddp-migrate-batch6-audit-callers.md.changeset/rest-audit-endpoints.mdapps/meteor/client/views/audit/components/AuditLogTable.tsxapps/meteor/client/views/audit/hooks/useAuditMutation.tsapps/meteor/ee/server/api/audit.tsapps/meteor/ee/server/lib/audit/functions.tsapps/meteor/ee/server/lib/audit/methods.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (7)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 📦 Meteor Build (coverage)
🧰 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/client/views/audit/components/AuditLogTable.tsxapps/meteor/client/views/audit/hooks/useAuditMutation.tsapps/meteor/ee/server/lib/audit/functions.tsapps/meteor/ee/server/lib/audit/methods.tsapps/meteor/ee/server/api/audit.ts
🧠 Learnings (7)
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.
Applied to files:
.changeset/ddp-migrate-batch6-audit-callers.md.changeset/rest-audit-endpoints.md
📚 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/audit/components/AuditLogTable.tsx
📚 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/views/audit/components/AuditLogTable.tsxapps/meteor/client/views/audit/hooks/useAuditMutation.tsapps/meteor/ee/server/lib/audit/functions.tsapps/meteor/ee/server/lib/audit/methods.tsapps/meteor/ee/server/api/audit.ts
📚 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/views/audit/hooks/useAuditMutation.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/views/audit/hooks/useAuditMutation.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/views/audit/hooks/useAuditMutation.tsapps/meteor/ee/server/lib/audit/functions.tsapps/meteor/ee/server/lib/audit/methods.tsapps/meteor/ee/server/api/audit.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/views/audit/hooks/useAuditMutation.tsapps/meteor/ee/server/lib/audit/functions.tsapps/meteor/ee/server/lib/audit/methods.tsapps/meteor/ee/server/api/audit.ts
🪛 ast-grep (0.44.0)
apps/meteor/ee/server/lib/audit/functions.ts
[warning] 131-131: Do not use variable for regular expressions
Context: new RegExp(escapeRegExp(msg).trim(), 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
[warning] 189-189: Do not use variable for regular expressions
Context: new RegExp(escapeRegExp(msg).trim(), 'i')
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🔇 Additional comments (6)
.changeset/ddp-migrate-batch6-audit-callers.md (1)
1-6: LGTM!.changeset/rest-audit-endpoints.md (1)
1-12: LGTM!apps/meteor/ee/server/lib/audit/functions.ts (1)
1-19: LGTM!Also applies to: 59-148, 206-227
apps/meteor/ee/server/lib/audit/methods.ts (1)
7-8: LGTM!apps/meteor/ee/server/api/audit.ts (2)
1-122: LGTM!Also applies to: 288-404
125-130: 🎯 Functional CorrectnessUse the 400 failure path for invalid dates.
parseDateOrFail()throws a plainError, while these endpoints declare 400 responses for validation failures. Add an explicitAPI.v1.failure(...)/Meteor.Errormapping here, or the route layer may turn malformed dates into 500s.
| queryFn: async () => { | ||
| const { start, end } = dateRange; | ||
| return getAudits({ startDate: start ?? new Date(0), endDate: end ?? new Date() }); | ||
| const { auditions } = await getAudits({ | ||
| startDate: (start ?? new Date(0)).toISOString(), | ||
| endDate: (end ?? new Date()).toISOString(), | ||
| }); | ||
| return auditions; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Deserialize the REST payload instead of force-casting it.
useEndpoint is returning the JSON response as-is here, but the table now treats each row as an IAuditLog via as unknown as IAuditLog. That only hides the contract mismatch from the DDP→REST migration: fields like ts and the audit filter dates will still be serialized when AuditLogEntry consumes them. useAuditMutation already maps REST messages back into client types; the audit-log path needs the same treatment before storing query data.
Suggested direction
+import { mapAuditLogFromApi } from '../../../lib/utils/mapAuditLogFromApi';
...
const { auditions } = await getAudits({
startDate: (start ?? new Date(0)).toISOString(),
endDate: (end ?? new Date()).toISOString(),
});
- return auditions;
+ return auditions.map(mapAuditLogFromApi);
...
- <AuditLogEntry key={auditLog._id} value={auditLog as unknown as IAuditLog} />
+ <AuditLogEntry key={auditLog._id} value={auditLog} />Also applies to: 72-72
🤖 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/audit/components/AuditLogTable.tsx` around lines 27
- 33, The audit log query is still force-casting the REST response into
IAuditLog instead of converting it to the client shape. Update AuditLogTable’s
useEndpoint/queryFn path to deserialize the payload before caching it, mapping
REST fields like ts and date values into the IAuditLog structure the table and
AuditLogEntry expect, similar to how useAuditMutation handles REST-to-client
conversion.
| if (type === 'l') { | ||
| return getOmnichannelAuditMessages({ | ||
| const { messages } = await getOmnichannelAuditMessages({ | ||
| type, | ||
| msg, | ||
| startDate: dateRange.start ?? new Date(0), | ||
| endDate: dateRange.end ?? new Date(), | ||
| startDate, | ||
| endDate, | ||
| users, | ||
| visitor: '', | ||
| agent: '', |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Forward the omnichannel visitor and agent filters.
This branch ignores the AuditFields values and always posts empty strings, so any omnichannel audit filtered by visitor or agent will return broader results than the user requested.
Minimal fix
const { messages } = await getOmnichannelAuditMessages({
type,
msg,
startDate,
endDate,
users,
- visitor: '',
- agent: '',
+ visitor,
+ 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.
| if (type === 'l') { | |
| return getOmnichannelAuditMessages({ | |
| const { messages } = await getOmnichannelAuditMessages({ | |
| type, | |
| msg, | |
| startDate: dateRange.start ?? new Date(0), | |
| endDate: dateRange.end ?? new Date(), | |
| startDate, | |
| endDate, | |
| users, | |
| visitor: '', | |
| agent: '', | |
| if (type === 'l') { | |
| const { messages } = await getOmnichannelAuditMessages({ | |
| type, | |
| msg, | |
| startDate, | |
| endDate, | |
| users, | |
| visitor, | |
| 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/client/views/audit/hooks/useAuditMutation.ts` around lines 19 -
27, The `useAuditMutation` branch for `type === 'l'` is ignoring the
`AuditFields` filter values by hardcoding `visitor` and `agent` to empty
strings. Update the `getOmnichannelAuditMessages` call to forward the actual
`visitor` and `agent` values from the current audit filters, using the existing
`AuditFields` data in `useAuditMutation` so omnichannel searches stay scoped
correctly.
| if (type === 'l') { | ||
| console.warn('Deprecation Warning! This method will be removed in the next version (4.0.0)'); | ||
| const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {}, { userId }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid emitting the stale DDP deprecation warning from shared REST logic.
This helper is now used by REST routes too, so this console.warn can log a misleading “method” deprecation with an old 4.0.0 removal target for non-DDP calls. Keep deprecation logging in the DDP wrappers only.
🤖 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/ee/server/lib/audit/functions.ts` around lines 44 - 46, The
shared audit helper in functions.ts is emitting a stale DDP-only deprecation
warning from logic now used by REST routes. Remove the console.warn from the
type === 'l' branch in the shared function and keep deprecation logging only in
the DDP wrapper path that calls this helper, so REST calls do not report a
misleading “method” warning or 4.0.0 removal target.
| const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, { | ||
| projection: { _id: 1 }, | ||
| }).toArray(); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Apply livechat room restrictions in the omnichannel audit path.
auditGetMessagesMethod applies livechat.applyRoomRestrictions before livechat room lookup, but this dedicated omnichannel method does not. That can bypass the same room-restriction filter for callers using /v1/audit.omnichannel.messages.
Suggested fix
+ const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {}, { userId: user._id });
const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, {
projection: { _id: 1 },
- }).toArray();
+ }, extraQuery).toArray();📝 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.
| const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, { | |
| projection: { _id: 1 }, | |
| }).toArray(); | |
| const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {}, { userId: user._id }); | |
| const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, { | |
| projection: { _id: 1 }, | |
| }, extraQuery).toArray(); |
🤖 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/ee/server/lib/audit/functions.ts` around lines 175 - 177, The
omnichannel audit path in auditGetMessagesMethod is missing the same livechat
room restriction filter used elsewhere, so apply livechat.applyRoomRestrictions
before the LivechatRooms.findByVisitorIdAndAgentId lookup. Update the audit
messages flow in functions.ts so the visitor/agent room search is constrained
consistently with the standard audit path, using the existing
auditGetMessagesMethod and livechat.applyRoomRestrictions symbols as the
insertion point.
| const rids = rooms?.length ? rooms.map(({ _id }) => _id) : undefined; | ||
| const name = i18n.t('Omnichannel'); | ||
|
|
||
| const query: Filter<IMessage> = { | ||
| rid: { $in: rids }, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline =="
ast-grep outline apps/meteor/ee/server/lib/audit/functions.ts --view expanded || true
echo
echo "== relevant slice =="
sed -n '140,230p' apps/meteor/ee/server/lib/audit/functions.ts
echo
echo "== search for similar patterns =="
rg -n "\$in: .*undefined|rooms\?\.length|Room doesn't exist|livechat room matches|rid: \{ \$in:" apps/meteor -g '*.ts' -g '*.js'Repository: RocketChat/Rocket.Chat
Length of output: 3876
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== top slice =="
sed -n '1,140p' apps/meteor/ee/server/lib/audit/functions.ts
echo
echo "== any tests around auditGetOmnichannelMessagesMethod =="
rg -n "auditGetOmnichannelMessagesMethod|Room doesn't exist|Omnichannel" apps/meteor -g '*.{ts,js}'Repository: RocketChat/Rocket.Chat
Length of output: 50379
🌐 Web query:
MongoDB $in operator requires array undefined behavior query error
💡 Result:
The MongoDB $in operator specifically requires the argument provided to it to be an array [1][2]. If this requirement is not met—for example, if you attempt to pass a non-array value or use the operator incorrectly within an aggregation pipeline—MongoDB will return an error stating that $in needs an array [3]. This is a structural requirement of the operator's syntax: { field: { $in: [ , ,... ] } } [1][2] Common causes for this error include: 1. Incorrect syntax in an aggregation pipeline where the $in operator might be receiving a scalar value instead of the expected array argument [3]. 2. Misunderstanding the operator's purpose; $in is intended for checking if a field's value matches any element in the provided array, not for checking if an array exists or contains a specific value in a way that allows for scalar arguments [1]. If you are encountering this error, ensure that the value passed to $in is wrapped in square brackets, forming a valid array, and that your query structure adheres to the official MongoDB operator specifications [1]. Note that this error is a functional validation error, not undefined behavior, as the engine explicitly enforces this input type [3]. Furthermore, while you should ensure your queries are syntactically correct, be aware that changes in recent MongoDB versions (starting in 8.0) have altered how queries interact with undefined or null values (e.g., equality matches no longer match undefined fields), which may require updates to how you handle such data in your predicates [4][5][6]. Always verify the specific MongoDB version you are running, as behavioral changes regarding BSON types can affect query results [5].
Citations:
- 1: https://www.mongodb.com/docs/manual/reference/operator/query/in/
- 2: https://www.mongodb.com/docs/v8.2/reference/operator/query/in/
- 3: https://stackoverflow.com/questions/60893951/mongodb-aggregation-error-in-needs-an-array
- 4: https://www.mongodb.com/docs/v8.2/reference/bson-types/migrate-undefined/
- 5: https://www.mongodb.com/docs/v8.2/release-notes/8.0-compatibility/
- 6: https://www.mongodb.com/docs/manual/reference/bson-types/migrate-undefined/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== top of audit/functions.ts =="
sed -n '1,140p' apps/meteor/ee/server/lib/audit/functions.ts
echo
echo "== exact room handling lines =="
nl -ba apps/meteor/ee/server/lib/audit/functions.ts | sed -n '1,90p'Repository: RocketChat/Rocket.Chat
Length of output: 4209
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== findByVisitorIdAndAgentId definitions/usages =="
rg -n "findByVisitorIdAndAgentId\(" apps/meteor packages -g '*.{ts,js}' | head -n 50
echo
echo "== audit omichannel method slice with line numbers =="
python3 - <<'PY'
from pathlib import Path
p = Path('apps/meteor/ee/server/lib/audit/functions.ts')
for i, line in enumerate(p.read_text().splitlines(), 1):
if 160 <= i <= 190:
print(f"{i:4d}: {line}")
PYRepository: RocketChat/Rocket.Chat
Length of output: 1747
Guard the empty livechat-room case before $in. LivechatRooms.findByVisitorIdAndAgentId() can return no rooms, which leaves rids as undefined and produces rid: { $in: undefined }; Mongo expects an array there and will reject the query. Throw/return before building query when no room matches.
🤖 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/ee/server/lib/audit/functions.ts` around lines 178 - 182, The
`findByVisitorIdAndAgentId`/audit query path should guard the no-room case
before building the Mongo filter. In the `rids`/`query` निर्माण, return or throw
early when `rooms` is empty so `rid: { $in: undefined }` is never created. Keep
the fix localized around the `rids` assignment and the `query: Filter<IMessage>`
construction.
| async auditGetOmnichannelMessages(params) { | ||
| methodDeprecationLogger.method('auditGetOmnichannelMessages', '9.0.0', '/v1/audit.omnichannel.messages'); | ||
| check(params.startDate, Date); | ||
| check(params.endDate, Date); | ||
| return auditGetOmnichannelMessagesMethod(Meteor.userId(), params); | ||
| }, | ||
| async auditGetMessages({ rid, startDate, endDate, users: usernames, msg, type, visitor, agent }) { | ||
| check(startDate, Date); | ||
| check(endDate, Date); | ||
|
|
||
| const user = (await Meteor.userAsync()) as IUser; | ||
| if (!user || !(await hasPermissionAsync(user._id, 'can-audit'))) { | ||
| throw new Meteor.Error('Not allowed'); | ||
| } | ||
|
|
||
| const userFields = { | ||
| _id: user._id, | ||
| username: user.username, | ||
| ...(user.name && { name: user.name }), | ||
| ...(user.avatarETag && { avatarETag: user.avatarETag }), | ||
| }; | ||
|
|
||
| let rids; | ||
| let name; | ||
|
|
||
| const query: Filter<IMessage> = { | ||
| ts: { | ||
| $gt: startDate, | ||
| $lt: endDate, | ||
| }, | ||
| }; | ||
|
|
||
| if (type === 'u') { | ||
| const usersId = await getUsersIdFromUserName(usernames); | ||
| query['u._id'] = { $in: usersId }; | ||
|
|
||
| const abacRooms = await Rooms.findAllPrivateRoomsWithAbacAttributes({ projection: { _id: 1 } }) | ||
| .map((doc) => doc._id) | ||
| .toArray(); | ||
|
|
||
| query.rid = { $nin: abacRooms }; | ||
| } else { | ||
| const roomInfo = await getRoomInfoByAuditParams({ type, roomId: rid, users: usernames, visitor, agent, userId: user._id }); | ||
| if (!roomInfo) { | ||
| throw new Meteor.Error(`Room doesn't exist`); | ||
| } | ||
|
|
||
| rids = roomInfo.rids; | ||
| name = roomInfo.name; | ||
| query.rid = { $in: rids }; | ||
| } | ||
|
|
||
| if (msg) { | ||
| const regex = new RegExp(escapeRegExp(msg).trim(), 'i'); | ||
| query.msg = regex; | ||
| } | ||
|
|
||
| const messages = await Messages.find(query).toArray(); | ||
|
|
||
| // Once the filter is applied, messages will be shown and a log containing all filters will be saved for further auditing. | ||
|
|
||
| await AuditLog.insertOne({ | ||
| ts: new Date(), | ||
| results: messages.length, | ||
| u: userFields, | ||
| fields: { msg, users: usernames, rids, room: name, startDate, endDate, type, visitor, agent }, | ||
| }); | ||
|
|
||
| updateCounter({ settingsId: 'Message_Auditing_Panel_Load_Count' }); | ||
|
|
||
| return messages; | ||
| async auditGetMessages(params) { | ||
| methodDeprecationLogger.method('auditGetMessages', '9.0.0', '/v1/audit.messages'); | ||
| check(params.startDate, Date); | ||
| check(params.endDate, Date); | ||
| return auditGetMessagesMethod(Meteor.userId(), params); | ||
| }, | ||
| async auditGetAuditions({ startDate, endDate }) { | ||
| methodDeprecationLogger.method('auditGetAuditions', '9.0.0', '/v1/audit.auditions'); | ||
| check(startDate, Date); | ||
| check(endDate, Date); | ||
| const uid = Meteor.userId(); | ||
| if (!uid || !(await hasPermissionAsync(uid, 'can-audit-log'))) { | ||
| throw new Meteor.Error('Not allowed'); | ||
| } | ||
| return AuditLog.find( | ||
| { | ||
| // 'u._id': userId, | ||
| ts: { | ||
| $gt: startDate, | ||
| $lt: endDate, | ||
| }, | ||
| }, | ||
| { | ||
| projection: { | ||
| 'u.services': 0, | ||
| 'u.roles': 0, | ||
| 'u.lastLogin': 0, | ||
| 'u.statusConnection': 0, | ||
| 'u.emails': 0, | ||
| }, | ||
| }, | ||
| ).toArray(); | ||
| return auditGetAuditionsMethod(Meteor.userId(), startDate, endDate); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the full DDP payload before delegating.
These DDP methods still accept untyped external input, but only the date fields are checked. Missing users, type, visitor, or agent can now reach the shared functions and fail deeper in query construction; validate the whole params object at the method boundary.
🤖 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/ee/server/lib/audit/methods.ts` around lines 37 - 53, The audit
DDP methods currently validate only the date fields, so untyped payloads can
still reach the shared audit query builders with missing users, type, visitor,
or agent data. Update the method boundaries in auditGetOmnichannelMessages,
auditGetMessages, and auditGetAuditions to validate the full params object
before calling auditGetOmnichannelMessagesMethod, auditGetMessagesMethod, and
auditGetAuditionsMethod. Keep the existing date checks, but add validation for
all required fields in each request shape so invalid input is rejected early.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 7 files
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/ee/server/lib/audit/functions.ts">
<violation number="1" location="apps/meteor/ee/server/lib/audit/functions.ts:45">
P3: This `console.warn` about DDP deprecation is now in shared logic called by both DDP methods and REST endpoints. When a REST caller hits this path with `type === 'l'`, it will log a misleading "This method will be removed in the next version (4.0.0)" warning. Move this deprecation logging to the DDP wrapper only.</violation>
<violation number="2" location="apps/meteor/ee/server/lib/audit/functions.ts:175">
P1: Apply livechat room restrictions before fetching omnichannel audit rooms. Otherwise restricted auditors can retrieve messages from rooms outside their allowed units/visibility.</violation>
<violation number="3" location="apps/meteor/ee/server/lib/audit/functions.ts:178">
P2: Keep rids as an array when no omnichannel rooms match. Passing undefined into $in can turn an empty result into a server error.</violation>
</file>
<file name="apps/meteor/ee/server/api/audit.ts">
<violation number="1" location="apps/meteor/ee/server/api/audit.ts:112">
P3: New REST endpoint typings omit the `success` field returned at runtime, so API consumers get a misleading contract for these migrated endpoints.
(Based on your team's feedback about keeping API typings aligned with runtime endpoints.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ...(user.avatarETag && { avatarETag: user.avatarETag }), | ||
| }; | ||
|
|
||
| const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, { |
There was a problem hiding this comment.
P1: Apply livechat room restrictions before fetching omnichannel audit rooms. Otherwise restricted auditors can retrieve messages from rooms outside their allowed units/visibility.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/ee/server/lib/audit/functions.ts, line 175:
<comment>Apply livechat room restrictions before fetching omnichannel audit rooms. Otherwise restricted auditors can retrieve messages from rooms outside their allowed units/visibility.</comment>
<file context>
@@ -0,0 +1,227 @@
+ ...(user.avatarETag && { avatarETag: user.avatarETag }),
+ };
+
+ const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, {
+ projection: { _id: 1 },
+ }).toArray();
</file context>
| const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, { | ||
| projection: { _id: 1 }, | ||
| }).toArray(); | ||
| const rids = rooms?.length ? rooms.map(({ _id }) => _id) : undefined; |
There was a problem hiding this comment.
P2: Keep rids as an array when no omnichannel rooms match. Passing undefined into $in can turn an empty result into a server error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/ee/server/lib/audit/functions.ts, line 178:
<comment>Keep rids as an array when no omnichannel rooms match. Passing undefined into $in can turn an empty result into a server error.</comment>
<file context>
@@ -0,0 +1,227 @@
+ const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(visitor, agent, {
+ projection: { _id: 1 },
+ }).toArray();
+ const rids = rooms?.length ? rooms.map(({ _id }) => _id) : undefined;
+ const name = i18n.t('Omnichannel');
+
</file context>
| const rids = rooms?.length ? rooms.map(({ _id }) => _id) : undefined; | |
| const rids = rooms.map(({ _id }) => _id); |
| } | ||
|
|
||
| if (type === 'l') { | ||
| console.warn('Deprecation Warning! This method will be removed in the next version (4.0.0)'); |
There was a problem hiding this comment.
P3: This console.warn about DDP deprecation is now in shared logic called by both DDP methods and REST endpoints. When a REST caller hits this path with type === 'l', it will log a misleading "This method will be removed in the next version (4.0.0)" warning. Move this deprecation logging to the DDP wrapper only.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/ee/server/lib/audit/functions.ts, line 45:
<comment>This `console.warn` about DDP deprecation is now in shared logic called by both DDP methods and REST endpoints. When a REST caller hits this path with `type === 'l'`, it will log a misleading "This method will be removed in the next version (4.0.0)" warning. Move this deprecation logging to the DDP wrapper only.</comment>
<file context>
@@ -0,0 +1,227 @@
+ }
+
+ if (type === 'l') {
+ console.warn('Deprecation Warning! This method will be removed in the next version (4.0.0)');
+ const extraQuery = await callbacks.run('livechat.applyRoomRestrictions', {}, { userId });
+ const rooms: IRoom[] = await LivechatRooms.findByVisitorIdAndAgentId(
</file context>
| @@ -1,4 +1,4 @@ | |||
| import type { IUser, IRoom } from '@rocket.chat/core-typings'; | |||
| import type { IAuditLog, IMessage, IUser, IRoom } from '@rocket.chat/core-typings'; | |||
There was a problem hiding this comment.
P3: New REST endpoint typings omit the success field returned at runtime, so API consumers get a misleading contract for these migrated endpoints.
(Based on your team's feedback about keeping API typings aligned with runtime endpoints.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/ee/server/api/audit.ts, line 112:
<comment>New REST endpoint typings omit the `success` field returned at runtime, so API consumers get a misleading contract for these migrated endpoints.
(Based on your team's feedback about keeping API typings aligned with runtime endpoints.) </comment>
<file context>
@@ -36,9 +107,29 @@ declare module '@rocket.chat/rest-typings' {
};
+
+ '/v1/audit.auditions': {
+ GET: (params: AuditAuditionsParams) => { auditions: IAuditLog[] };
+ };
+
</file context>
1 participant
Summary
Continues the DDP→REST sweep (#40659, #40711, #40675, #40724, #40728, #40734). This batch migrates the three
auditGet*DDP methods that backed the EE audit panel. DDP methods stay registered for external SDK/mobile clients with deprecation logs pointing at the new routes.New endpoints (EE —
auditinglicense required)auditGetAuditionsGET /v1/audit.auditionscan-audit-log?startDate=&endDate=(ISO)auditGetMessagesPOST /v1/audit.messagescan-audit{ rid?, startDate, endDate, users, msg, type, visitor?, agent? }auditGetOmnichannelMessagesPOST /v1/audit.omnichannel.messagescan-audit{ startDate, endDate, users, msg, type, visitor?, agent? }Each endpoint is rate-limited at 10/60s (matching the DDP
DDPRateLimiterrules) and writes the sameAuditLogentry the DDP path produced. Dates are serialized as ISO strings on the wire.Implementation
apps/meteor/ee/server/lib/audit/functions.ts; DDP entrypoints become thin + deprecation-logged.messagesandomnichannel.messagesarePOSTbecause the audit log insertion is a side effect (write) and the query params include arrays (users) that don't serialize cleanly into query strings.Client changes
AuditLogTable→useEndpoint('GET', '/v1/audit.auditions')useAuditMutation→useEndpoint('POST', '/v1/audit.messages')+useEndpoint('POST', '/v1/audit.omnichannel.messages').toISOString().Test plan
can-audit-logperm) → past entries rendercurland confirm 403 without license/permission, 200 with both🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores