feat: (poc) Unified AI Search#40890
Open
Dnouv wants to merge 55 commits into
Open
Conversation
# Conflicts: # apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts # packages/rest-typings/src/v1/misc.ts
coderabbitai
Bot
added
the
type: feature
![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: 11
🧹 Nitpick comments (5)
apps/meteor/client/views/search/SearchPage.tsx (1)
1-1: 🏗️ Heavy liftRemove the file-level ESLint disable by extracting components instead of suppressing the rule globally.
Line 1 disables
react/no-multi-compfor the entire file; please splitSourceResult/AnswerPanelinto separate modules and drop the comment-level suppression.
As per coding guidelines,**/*.{ts,tsx,js}: “Avoid code comments in the implementation”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/views/search/SearchPage.tsx` at line 1, Remove the file-level ESLint disable comment `/* eslint-disable react/no-multi-comp */` from the top of SearchPage.tsx. Instead of using this global suppression, extract the SourceResult and AnswerPanel components from SearchPage.tsx into their own separate module files. This follows the coding guideline to avoid code comments in implementation and ensures each component has proper module organization rather than relying on lint rule suppressions.Source: Coding guidelines
apps/meteor/client/views/admin/aiCenter/AICenterRoute.tsx (1)
1-1: ⚡ Quick winRemove the file-level lint-disable comment by splitting components.
/* eslint-disable react/no-multi-comp */violates the “no implementation comments” rule and suppresses structure feedback for the entire module. Please extract the local components into separate files and drop this disable.
As per coding guidelines,**/*.{ts,tsx,js}should “Avoid code comments in the implementation”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/meteor/client/views/admin/aiCenter/AICenterRoute.tsx` at line 1, Remove the file-level eslint disable comment at the top of AICenterRoute.tsx that suppresses the react/no-multi-comp rule. To do this, identify all local components defined within this file, extract each into its own separate file in the same directory, and then import them back into AICenterRoute.tsx. This eliminates the need for the disable comment and follows the coding guideline of avoiding implementation comments in source files.Source: Coding guidelines
apps/meteor/tests/unit/server/services/ai-search/service.tests.ts (1)
200-257: ⚡ Quick winAdd a regression test for non-visible
msg_idcandidates.Please add a
searchtest that asserts: when pipeline returns a candidate withmetadata.msg_id, butMessages.findVisibleByIdsreturns no matching message, the result is dropped (not rendered from pipeline text). This protects the visibility 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/tests/unit/server/services/ai-search/service.tests.ts` around lines 200 - 257, Add a new test case (using `it()`) in the test suite that verifies the visibility boundary protection. Configure the test to have `serverFetch` return a pipeline result with a candidate message ID, but set `Messages.findVisibleByIds.returns(cursor([]))` to return an empty cursor so the message is not visible to the user. Call `createService().search()` with appropriate filters and userId, then assert that the results array has length 0 to confirm that non-visible messages are dropped and not rendered from pipeline text, protecting the visibility boundary as intended.packages/ai-search/src/intelligentSearch.spec.ts (1)
30-49: ⚡ Quick winAdd a regression case for nullable
similarityvalues.Please add a test where
similarityisnull/''whiledistanceis numeric, and assert distance fallback still drives the normalized score.🤖 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 `@packages/ai-search/src/intelligentSearch.spec.ts` around lines 30 - 49, Add a regression test case to the normalizeIntelligentSearchCandidates test that verifies correct score calculation when similarity is null or empty string while distance is numeric. Within the test input array in normalizeIntelligentSearchCandidates, include an object with a numeric distance value but similarity set to null or empty string, and add a corresponding assertion in the expected results array that validates the distance value was correctly used as the fallback to compute the normalized score.packages/ai-search/src/llm.spec.ts (1)
74-113: ⚡ Quick winCover thrown provider exceptions in answer-generation tests.
Please add a case where
fetchrejects (and optionally wherejson()throws) and assert rejection witherror-ai-provider-request-failed.🤖 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 `@packages/ai-search/src/llm.spec.ts` around lines 74 - 113, The test for the generateOpenAICompatibleSearchAnswer function in the 'throws on provider failures and empty responses' block currently covers failed HTTP responses and empty responses, but does not cover cases where the fetch operation itself rejects or throws. Add test cases where the AIServiceFetch function rejects with an error (simulating network failures) and optionally where the json() method throws an exception. Both scenarios should call generateOpenAICompatibleSearchAnswer with the same parameters as existing test cases and assert that the promise rejects with 'error-ai-provider-request-failed', ensuring that thrown provider exceptions are properly caught and converted to the appropriate error code.
🤖 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/api/server/v1/ai-search.ts`:
- Around line 250-255: The unified endpoint accepts and parses `rids`,
`roomNames`, and `fromUsernames` filters, but the message search filtering only
forwards a subset of these filters (fromUsername, startDate, endDate), causing a
contract mismatch. Update all locations where messageSearch is called (at lines
261-263 and 287) to include the complete set of filters: rids, roomNames,
fromUsernames, fromUsername, startDate, and endDate. Ensure the message search
implementation handles all these filter parameters consistently with how
intelligent results handle them, so that message results respect the same filter
contract as the unified endpoint declares.
- Around line 242-243: After trimming the query string in the handler at the
point where const query is assigned the trimmed value, add a validation check to
reject the request if the query becomes empty after trimming. This ensures that
whitespace-only inputs (like " ") are properly rejected instead of proceeding
as empty semantic queries and triggering expensive downstream operations. The
check should validate that query.length is greater than 0 after the trim() call
and throw an appropriate error response if validation fails.
In `@apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts`:
- Around line 342-344: The Set deduplication on line 343 in the rooms return
statement only removes duplicate references, not duplicate rooms with the same
rid or _id that come from different sources (exact, localRooms,
resultsFromServer). Replace the Set-based approach with a key-based
deduplication strategy that uses the rid or _id field to ensure rooms from
different sources that represent the same entity are only included once in the
final array.
In `@apps/meteor/server/services/ai-search/service.ts`:
- Around line 214-226: When a result has a msgId that exists but the message is
not found in the messageMap (indicating it's deleted, hidden, or not visible),
the code should drop that candidate entirely rather than falling back to
result.pipelineText. Add a check after retrieving dbMessage on line 214 to
verify that if result.msgId exists, dbMessage must also be defined; if
result.msgId exists but dbMessage is undefined, return an empty array
immediately. This prevents leaking indexed text for non-visible messages and
ensures the fallback to result.pipelineText only occurs when msgId was not
provided in the original result.
In `@docs/intelligent-search-core-vs-apps-engine.md`:
- Around line 55-56: Update the documentation reference from `getActiveFilter`
to `getActiveSearchFilter` to match the actual implemented helper function name.
This change should be made in the docs where it currently states that
`getActiveFilter` detects in-progress tokens and shows dropdown autocomplete,
ensuring the documentation accurately reflects the actual API surface name.
- Line 37: Add language identifiers to all fenced code blocks in the file
docs/intelligent-search-core-vs-apps-engine.md to comply with markdownlint rule
MD040. At line 37 (anchor), add the appropriate language identifier after the
opening triple backticks. Apply the same fix to the sibling locations at lines
62, 205, 288, 325, 337, and 519. For each code block, determine the correct
language tag based on the code content (for example, json, yaml, python,
javascript, etc.) and place it immediately after the opening triple backticks
without any spaces.
In `@packages/ai-search/src/intelligentSearch.ts`:
- Around line 246-249: Remove the raw response body from the warn log in the
error handling block when the upstream response is not okay. Instead of logging
the body content with body.slice(0, 500), log only the status code and
optionally the body length or other non-sensitive metadata. Apply this same
redaction pattern to all similar error logging calls where upstream response
bodies are being captured and logged, to ensure user query and result data
fragments are not persisted in logs.
- Around line 26-33: The firstNumber function is coercing nullable and falsy
values (null, empty strings, false) into numeric zero via the Number()
conversion, which incorrectly overrides real score fields. Modify the loop in
firstNumber to skip null, empty string, and false values before attempting the
Number() conversion. Add explicit type and value checks to ensure only
legitimate numeric values or numeric strings are processed, preventing falsy
values from being coerced into zero scores.
In `@packages/ai-search/src/llm.ts`:
- Around line 48-76: The fetch request and response.json() calls in this block
can throw errors that currently escape unnormalized, while non-2xx responses are
already being mapped to error-ai-provider-request-failed. Wrap the entire fetch
operation (from the initial fetch call through response.json()) in a try-catch
block that catches any thrown errors and re-throws them as
error-ai-provider-request-failed to ensure consistent error handling across all
failure modes. Keep the existing check for empty answer separate, as it
represents a valid response with invalid content.
In `@packages/i18n/src/locales/en.i18n.json`:
- Line 181: The value for the "Intelligent_Search_start_from_top_bar" key in the
en.i18n.json file contains awkward phrasing with "search semantic workspace
knowledge" which feels unclear and unnatural. Replace this phrase with a more
natural and clear alternative such as "search your workspace semantically" to
improve the readability and user experience of the AI Search entry-point hint
message.
- Line 194: The description for the AI_Thread_Summarization_Enabled_Description
field in the en.i18n.json file is vague and does not clearly communicate what
the setting does or how it affects behavior. Replace the current text with a
clear, action-oriented description that explicitly explains what enabling this
setting allows (e.g., enables the system to generate summaries, allows AI to
summarize threads, etc.) to help administrators understand the actual
functionality and impact when toggling this feature.
---
Nitpick comments:
In `@apps/meteor/client/views/admin/aiCenter/AICenterRoute.tsx`:
- Line 1: Remove the file-level eslint disable comment at the top of
AICenterRoute.tsx that suppresses the react/no-multi-comp rule. To do this,
identify all local components defined within this file, extract each into its
own separate file in the same directory, and then import them back into
AICenterRoute.tsx. This eliminates the need for the disable comment and follows
the coding guideline of avoiding implementation comments in source files.
In `@apps/meteor/client/views/search/SearchPage.tsx`:
- Line 1: Remove the file-level ESLint disable comment `/* eslint-disable
react/no-multi-comp */` from the top of SearchPage.tsx. Instead of using this
global suppression, extract the SourceResult and AnswerPanel components from
SearchPage.tsx into their own separate module files. This follows the coding
guideline to avoid code comments in implementation and ensures each component
has proper module organization rather than relying on lint rule suppressions.
In `@apps/meteor/tests/unit/server/services/ai-search/service.tests.ts`:
- Around line 200-257: Add a new test case (using `it()`) in the test suite that
verifies the visibility boundary protection. Configure the test to have
`serverFetch` return a pipeline result with a candidate message ID, but set
`Messages.findVisibleByIds.returns(cursor([]))` to return an empty cursor so the
message is not visible to the user. Call `createService().search()` with
appropriate filters and userId, then assert that the results array has length 0
to confirm that non-visible messages are dropped and not rendered from pipeline
text, protecting the visibility boundary as intended.
In `@packages/ai-search/src/intelligentSearch.spec.ts`:
- Around line 30-49: Add a regression test case to the
normalizeIntelligentSearchCandidates test that verifies correct score
calculation when similarity is null or empty string while distance is numeric.
Within the test input array in normalizeIntelligentSearchCandidates, include an
object with a numeric distance value but similarity set to null or empty string,
and add a corresponding assertion in the expected results array that validates
the distance value was correctly used as the fallback to compute the normalized
score.
In `@packages/ai-search/src/llm.spec.ts`:
- Around line 74-113: The test for the generateOpenAICompatibleSearchAnswer
function in the 'throws on provider failures and empty responses' block
currently covers failed HTTP responses and empty responses, but does not cover
cases where the fetch operation itself rejects or throws. Add test cases where
the AIServiceFetch function rejects with an error (simulating network failures)
and optionally where the json() method throws an exception. Both scenarios
should call generateOpenAICompatibleSearchAnswer with the same parameters as
existing test cases and assert that the promise rejects with
'error-ai-provider-request-failed', ensuring that thrown provider exceptions are
properly caught and converted to the appropriate error code.
🪄 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: b142c439-25ec-4c6f-aa7b-b1e5075168c9
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
.changeset/fancy-deserts-mate.mdapps/meteor/app/api/server/index.tsapps/meteor/app/api/server/v1/ai-search.tsapps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsxapps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsxapps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsxapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tsapps/meteor/client/startup/routes.tsxapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/client/views/admin/routes.tsxapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/client/views/admin/sidebarItems.tsapps/meteor/client/views/search/SearchPage.spec.tsxapps/meteor/client/views/search/SearchPage.tsxapps/meteor/jest.config.tsapps/meteor/package.jsonapps/meteor/server/methods/messageSearch.tsapps/meteor/server/services/ai-search/service.tsapps/meteor/server/services/startup.tsapps/meteor/server/settings/ai.tsapps/meteor/server/settings/index.tsapps/meteor/tests/end-to-end/api/miscellaneous.tsapps/meteor/tests/end-to-end/api/settings.tsapps/meteor/tests/unit/server/services/ai-search/service.tests.tsdocs/intelligent-search-core-vs-apps-engine.mdpackages/ai-search/README.mdpackages/ai-search/jest.config.tspackages/ai-search/package.jsonpackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/clientSearch.tspackages/ai-search/src/constants.tspackages/ai-search/src/index.tspackages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/intelligentSearch.tspackages/ai-search/src/llm.spec.tspackages/ai-search/src/llm.tspackages/ai-search/src/types.tspackages/ai-search/src/url.tspackages/ai-search/tsconfig.jsonpackages/core-services/src/index.tspackages/core-services/src/types/IAISearchService.tspackages/i18n/src/locales/en.i18n.jsonpackages/rest-typings/src/v1/misc.tspackages/ui-client/src/hooks/useFeaturePreviewList.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). (10)
- GitHub Check: 🚢 Build Docker (amd64, authorization-service, queue-worker-service, ddp-streamer-service, cove...
- GitHub Check: 🚢 Build Docker (arm64, rocketchat, coverage)
- GitHub Check: 🚢 Build Docker (arm64, account-service, presence-service, omnichannel-transcript-service, cove...
- GitHub Check: 🚢 Build Docker (amd64, account-service, presence-service, omnichannel-transcript-service, cove...
- GitHub Check: 🚢 Build Docker (arm64, authorization-service, queue-worker-service, ddp-streamer-service, cove...
- GitHub Check: 🚢 Build Docker (amd64, rocketchat, coverage)
- GitHub Check: cubic · AI code reviewer
- GitHub Check: Hacktron Security Check
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/settings/index.tspackages/ai-search/jest.config.tspackages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/index.tsapps/meteor/app/api/server/index.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/views/admin/sidebarItems.tspackages/ai-search/src/constants.tsapps/meteor/jest.config.tsapps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsxpackages/ai-search/src/url.tsapps/meteor/server/services/startup.tspackages/ai-search/src/types.tspackages/core-services/src/index.tsapps/meteor/client/startup/routes.tsxpackages/ai-search/src/llm.tsapps/meteor/server/settings/ai.tsapps/meteor/client/views/admin/routes.tsxapps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsxpackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.tsapps/meteor/app/api/server/v1/ai-search.tsapps/meteor/client/views/search/SearchPage.spec.tsxapps/meteor/client/views/search/SearchPage.tsxpackages/ai-search/src/intelligentSearch.tsapps/meteor/tests/end-to-end/api/settings.tsapps/meteor/tests/unit/server/services/ai-search/service.tests.tsapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/server/methods/messageSearch.tspackages/ai-search/src/clientSearch.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tspackages/rest-typings/src/v1/misc.tsapps/meteor/tests/end-to-end/api/miscellaneous.tsapps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsxapps/meteor/server/services/ai-search/service.tspackages/core-services/src/types/IAISearchService.ts
**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.spec.ts: Use descriptive test names that clearly communicate expected behavior in Playwright tests
Use.spec.tsextension for test files (e.g.,login.spec.ts)
Files:
packages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.ts
🧠 Learnings (15)
📚 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/settings/index.tspackages/ai-search/jest.config.tspackages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/index.tsapps/meteor/app/api/server/index.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/views/admin/sidebarItems.tspackages/ai-search/src/constants.tsapps/meteor/jest.config.tspackages/ai-search/src/url.tsapps/meteor/server/services/startup.tspackages/ai-search/src/types.tspackages/core-services/src/index.tspackages/ai-search/src/llm.tsapps/meteor/server/settings/ai.tspackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.tsapps/meteor/app/api/server/v1/ai-search.tspackages/ai-search/src/intelligentSearch.tsapps/meteor/tests/end-to-end/api/settings.tsapps/meteor/tests/unit/server/services/ai-search/service.tests.tsapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/server/methods/messageSearch.tspackages/ai-search/src/clientSearch.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tspackages/rest-typings/src/v1/misc.tsapps/meteor/tests/end-to-end/api/miscellaneous.tsapps/meteor/server/services/ai-search/service.tspackages/core-services/src/types/IAISearchService.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/settings/index.tspackages/ai-search/jest.config.tspackages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/index.tsapps/meteor/app/api/server/index.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/views/admin/sidebarItems.tspackages/ai-search/src/constants.tsapps/meteor/jest.config.tspackages/ai-search/src/url.tsapps/meteor/server/services/startup.tspackages/ai-search/src/types.tspackages/core-services/src/index.tspackages/ai-search/src/llm.tsapps/meteor/server/settings/ai.tspackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.tsapps/meteor/app/api/server/v1/ai-search.tspackages/ai-search/src/intelligentSearch.tsapps/meteor/tests/end-to-end/api/settings.tsapps/meteor/tests/unit/server/services/ai-search/service.tests.tsapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/server/methods/messageSearch.tspackages/ai-search/src/clientSearch.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tspackages/rest-typings/src/v1/misc.tsapps/meteor/tests/end-to-end/api/miscellaneous.tsapps/meteor/server/services/ai-search/service.tspackages/core-services/src/types/IAISearchService.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/settings/index.tspackages/ai-search/jest.config.tspackages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/index.tsapps/meteor/app/api/server/index.tspackages/ui-client/src/hooks/useFeaturePreviewList.tsapps/meteor/client/views/admin/sidebarItems.tspackages/ai-search/src/constants.tsapps/meteor/jest.config.tsapps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsxpackages/ai-search/src/url.tsapps/meteor/server/services/startup.tspackages/ai-search/src/types.tspackages/core-services/src/index.tsapps/meteor/client/startup/routes.tsxpackages/ai-search/src/llm.tsapps/meteor/server/settings/ai.tsapps/meteor/client/views/admin/routes.tsxapps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsxpackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.tsapps/meteor/app/api/server/v1/ai-search.tsapps/meteor/client/views/search/SearchPage.spec.tsxapps/meteor/client/views/search/SearchPage.tsxpackages/ai-search/src/intelligentSearch.tsapps/meteor/tests/end-to-end/api/settings.tsapps/meteor/tests/unit/server/services/ai-search/service.tests.tsapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/server/methods/messageSearch.tspackages/ai-search/src/clientSearch.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tspackages/rest-typings/src/v1/misc.tsapps/meteor/tests/end-to-end/api/miscellaneous.tsapps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsxapps/meteor/server/services/ai-search/service.tspackages/core-services/src/types/IAISearchService.ts
📚 Learning: 2025-12-10T21:00:43.645Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:43.645Z
Learning: Adopt the monorepo-wide Jest testMatch pattern: <rootDir>/src/**/*.spec.{ts,js,mjs} (represented here as '**/src/**/*.spec.{ts,js,mjs}') to ensure spec files under any package's src directory are picked up consistently across all packages in the Rocket.Chat monorepo. Apply this pattern in jest.config.ts for all relevant packages to maintain uniform test discovery.
Applied to files:
packages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.ts
📚 Learning: 2026-02-24T19:22:48.358Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 38493
File: apps/meteor/tests/e2e/omnichannel/omnichannel-send-pdf-transcript.spec.ts:66-67
Timestamp: 2026-02-24T19:22:48.358Z
Learning: In Playwright end-to-end tests (e.g., under apps/meteor/tests/e2e/...), prefer locating elements by translated text (getByText) and ARIA roles (getByRole) over data-qa attributes. If translation values change, update the corresponding test locators accordingly. Never use data-qa locators. This guideline applies to all Playwright e2e test specs in the repository and helps keep tests robust to UI text changes and accessible semantics.
Applied to files:
packages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.ts
📚 Learning: 2026-03-06T18:10:15.268Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 39397
File: packages/gazzodown/src/code/CodeBlock.spec.tsx:47-68
Timestamp: 2026-03-06T18:10:15.268Z
Learning: In tests (especially those using testing-library/dom/jsdom) for Rocket.Chat components, the HTML <code> element has an implicit ARIA role of 'code'. Therefore, screen.getByRole('code') or screen.findByRole('code') will locate <code> elements even without a role attribute. Do not flag findByRole('code') as invalid in reviews; prefer using the implicit role instead of adding role="code" unless necessary for accessibility.
Applied to files:
packages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/clientSearch.spec.tspackages/ai-search/src/llm.spec.tsapps/meteor/client/views/search/SearchPage.spec.tsx
📚 Learning: 2026-01-19T18:08:05.314Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38169
File: packages/ui-client/src/hooks/useGoToDirectMessage.ts:20-27
Timestamp: 2026-01-19T18:08:05.314Z
Learning: Rule of Hooks: In React, hooks (including custom hooks like useGoToDirectMessage and useUserSubscriptionByName) must be called unconditionally and in the same order on every render. Do not conditionally call hooks based on values; instead, derive safe inputs if a value may be undefined (e.g., pass an empty string or undefined) and handle variations in logic outside the hook invocation. This preserves hook order and avoids violations. Apply this guideline to all files under packages/ui-client/src/hooks where hooks are used.
Applied to files:
packages/ui-client/src/hooks/useFeaturePreviewList.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/admin/sidebarItems.tsapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.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/admin/sidebarItems.tsapps/meteor/client/views/admin/settings/hooks/useSettingsGroups.tsapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsxapps/meteor/client/startup/routes.tsxapps/meteor/client/views/admin/routes.tsxapps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsxapps/meteor/client/views/search/SearchPage.spec.tsxapps/meteor/client/views/search/SearchPage.tsxapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx
📚 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/fancy-deserts-mate.md
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/ai-search.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/ai-search.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/messageSearch.ts
📚 Learning: 2026-05-11T23:14:59.316Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 40469
File: packages/rest-typings/src/v1/users.ts:337-337
Timestamp: 2026-05-11T23:14:59.316Z
Learning: In Rocket.Chat REST endpoint typings (e.g., packages/rest-typings/src/v1/users.ts and other rest-typings files), keep the established convention of deriving field types from the domain model (e.g., use IUser indexed access like IUser['statusExpiresAt']) rather than swapping individual fields to serialized primitives (like string) in an ad-hoc way. If a truly different “serialized” representation is needed, perform the refactor consistently across the codebase (not just a single endpoint/field) and ensure all related REST typings stay aligned with the shared serialization types.
Applied to files:
packages/rest-typings/src/v1/misc.ts
🪛 LanguageTool
docs/intelligent-search-core-vs-apps-engine.md
[style] ~406-~406: ‘out of reach’ might be wordy. Consider a shorter alternative.
Context: ...age layout, and license integration are out of reach). --- ### Option C — Apps Engine app ...
(EN_WORDINESS_PREMIUM_OUT_OF_REACH)
[uncategorized] ~532-~532: Do not mix variants of the same word (‘normalization’ and ‘normalisation’) within a single text.
Context: ...ipeline filters, pipeline calls, result normalization, OpenAI-compatible answers, and model l...
(EN_WORD_COHERENCY)
[grammar] ~544-~544: Use a hyphen to join words.
Context: ...e vector pipeline, held for up to the 10 second timeout - one outbound HTTP conne...
(QB_NEW_EN_HYPHEN)
[grammar] ~545-~545: Use a hyphen to join words.
Context: ...M answer provider, held for up to the 20 second timeout The MongoDB queries with...
(QB_NEW_EN_HYPHEN)
[style] ~626-~626: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...SE_EXTERNAL_AI_SEARCH_SERVICE=true` | | Very large / multi-node deployment | Scale AI Sear...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.22.1)
docs/intelligent-search-core-vs-apps-engine.md
[warning] 37-37: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 205-205: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 325-325: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 337-337: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 519-519: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 OpenGrep (1.22.0)
packages/ai-search/src/clientSearch.ts
[ERROR] 149-149: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔇 Additional comments (35)
apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx (1)
33-238: LGTM!apps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx (1)
3-166: LGTM!apps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsx (1)
1-81: LGTM!apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts (1)
1-341: LGTM!Also applies to: 345-357
.changeset/fancy-deserts-mate.md (1)
1-12: LGTM!apps/meteor/client/startup/routes.tsx (1)
27-27: LGTM!Also applies to: 115-118, 249-257
apps/meteor/client/views/search/SearchPage.tsx (1)
2-430: LGTM!apps/meteor/client/views/search/SearchPage.spec.tsx (1)
1-42: LGTM!apps/meteor/jest.config.ts (1)
26-27: LGTM!apps/meteor/package.json (1)
96-96: LGTM!apps/meteor/client/views/admin/aiCenter/AICenterRoute.tsx (1)
2-147: LGTM!apps/meteor/client/views/admin/routes.tsx (1)
111-114: LGTM!Also applies to: 199-202
apps/meteor/client/views/admin/settings/hooks/useSettingsGroups.ts (1)
2-2: LGTM!Also applies to: 20-21
apps/meteor/client/views/admin/sidebarItems.ts (1)
49-56: LGTM!packages/ui-client/src/hooks/useFeaturePreviewList.ts (1)
3-3: LGTM!Also applies to: 9-9, 40-47
packages/ai-search/package.json (1)
1-31: LGTM!packages/ai-search/tsconfig.json (1)
1-9: LGTM!packages/core-services/src/types/IAISearchService.ts (1)
1-51: LGTM!packages/core-services/src/index.ts (1)
2-9: LGTM!Also applies to: 157-163, 221-221
apps/meteor/server/services/startup.ts (1)
8-8: LGTM!Also applies to: 66-68
packages/ai-search/jest.config.ts (1)
1-6: LGTM!packages/ai-search/src/constants.ts (1)
1-13: LGTM!packages/ai-search/src/index.ts (1)
1-6: LGTM!packages/ai-search/src/clientSearch.ts (1)
1-258: LGTM!packages/ai-search/src/clientSearch.spec.ts (1)
1-165: LGTM!apps/meteor/server/settings/ai.ts (1)
1-143: LGTM!apps/meteor/server/settings/index.ts (1)
2-2: LGTM!Also applies to: 43-43
packages/ai-search/README.md (1)
1-17: LGTM!packages/ai-search/src/types.ts (1)
1-81: LGTM!packages/ai-search/src/url.ts (1)
1-9: LGTM!packages/rest-typings/src/v1/misc.ts (1)
1-1: LGTM!Also applies to: 83-86, 99-249, 388-415
apps/meteor/server/methods/messageSearch.ts (1)
1-1: LGTM!Also applies to: 24-28, 36-37, 100-118
apps/meteor/app/api/server/index.ts (1)
10-10: LGTM!apps/meteor/tests/end-to-end/api/miscellaneous.ts (1)
741-814: LGTM!apps/meteor/tests/end-to-end/api/settings.ts (1)
104-104: LGTM!Also applies to: 116-117
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
15 issues found across 45 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/server/settings/index.ts">
<violation number="1" location="apps/meteor/server/settings/index.ts:43">
P3: AI settings now include `AI_Thread_Summarization`, but the AI Center UI only routes to Intelligent_Search and AI_LLM_Provider. That leaves the new section unreachable from the feature preview entrypoint.</violation>
</file>
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}
There was a problem hiding this comment.
An authorization bypass vulnerability exists in the Rocket.Chat spotlight search feature, allowing any registered user to disclose the membership list of any private room or private team on the server.
Root Cause
In Spotlight.searchUsers, the variable canListOutsiders checks if the requester has the permissions view-outside-room and view-d-room (which are standard permissions granted to the default user role to allow starting direct messages with other users globally).
However, the variable canListInsiders is computed as:
const canListInsiders = canListOutsiders || (rid && (await canAccessRoomAsync(room, { _id: userId })));
Because canListOutsiders is true for standard users, canListInsiders is evaluated as true even if the user is not a member of the room and canAccessRoomAsync returns false.
If canListInsiders is true and a room ID (rid) is provided, the search logic queries for users belonging to that specific room (__rooms: rid) via _searchInsiderUsers. This allows any authenticated user to search for and retrieve the list of users inside any private room, bypassing the room access authorization check.
Preconditions
- The attacker must have a registered account on the Rocket.Chat instance (possessing the default
userrole withview-outside-roomandview-d-roompermissions). - The attacker must obtain or guess the target private room's ID (
rid).
Exploitation Path
- The attacker registers a standard user account on the target Rocket.Chat instance.
- The attacker identifies the
ridof a target private room or private team. - The attacker calls the
spotlightAPI endpoint (or invokes the DDPspotlightmethod) with parametersrid=<target_rid>andquery=<a>. - The server returns matching users who are members of the private room.
- By repeating this query with different search terms, the attacker can completely reconstruct the membership list of the private room.
Security Impact
This vulnerability leads to a severe disclosure of sensitive information. Private rooms are designed to restrict visibility of their existence and membership to authorized members only. Leaking the identity of users in private channels (e.g., executive, legal, HR, or security-related channels) violates the core trust boundaries of the platform.
Steps to Reproduce
An attacker can send the following HTTP request using their authentication token to retrieve users inside a target private room:
curl -H "X-Auth-Token: <your_auth_token>" \
-H "X-User-Id: <your_user_id>" \
"http://localhost:3000/api/v1/spotlight?query=a&rid=<private_room_id>&type=users"The response will contain the list of users belonging to the private room whose names or usernames match the query "a".
Trace
graph TD
subgraph SG0 ["apps/meteor/app/api/server/v1/ai-search.ts"]
action_2["Handles requests for generating AI-based search answers."]
API.get.arg3_2["Unified search endpoint handler for users, rooms, messages, and AI."]
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG1 ["apps/meteor/app/api/server/v1/misc.ts"]
action["action"]
API.get.arg3["API.get.arg3"]
end
style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG2 ["apps/meteor/app/authorization/server/functions/hasPermission.ts"]
hasAllPermissionAsync["hasAllPermissionAsync"]
end
style SG2 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG3 ["apps/meteor/app/authorization/server/index.ts"]
._Rocket.Chat_apps_meteor_app_authorization_server_index.ts["./Rocket.Chat/apps/meteor/app/authorization/server/index.ts"]
end
style SG3 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG4 ["apps/meteor/app/settings/server/CachedSettings.ts"]
CachedSettings.get["Retrieves a setting value from the cache."]
end
style SG4 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG5 ["apps/meteor/lib/utils/stringUtils.ts"]
makeString["makeString"]
defaultToWhiteSpace["defaultToWhiteSpace"]
trim["trim"]
end
style SG5 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG6 ["apps/meteor/server/database/readSecondaryPreferred.ts"]
readSecondaryPreferred["Utility to determine MongoDB read preference for secondary preferred."]
end
style SG6 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG7 ["apps/meteor/server/lib/spotlight.js"]
Spotlight.mapOutsiders["Spotlight.mapOutsiders"]
Spotlight.processLimitAndUsernames["Spotlight.processLimitAndUsernames"]
Spotlight._searchInsiderUsers["Spotlight._searchInsiderUsers"]
Spotlight._searchConnectedUsers["Spotlight._searchConnectedUsers"]
Spotlight._searchOutsiderUsers["Spotlight._searchOutsiderUsers"]
Spotlight.mapTeams["Spotlight.mapTeams"]
Spotlight._searchTeams["Spotlight._searchTeams"]
Spotlight.searchUsers{{"Searches for users across the system, including insiders and outsiders, based on a text query and room context."}}
end
style SG7 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG8 ["apps/meteor/server/publications/spotlight.ts"]
spotlightMethod["Orchestrates the spotlight search process by querying users and rooms."]
spotlight["spotlight"]
end
style SG8 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG9 ["apps/meteor/server/services/authorization/service.ts"]
Authorization.hasAllPermission["Authorization.hasAllPermission"]
Authorization.all["Authorization.all"]
end
style SG9 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG10 ["packages/models/src/models/BaseRaw.ts"]
BaseRaw.doNotMixInclusionAndExclusionFields["BaseRaw.doNotMixInclusionAndExclusionFields"]
BaseRaw.ensureDefaultFields["BaseRaw.ensureDefaultFields"]
BaseRaw.find["BaseRaw.find"]
end
style SG10 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG11 ["packages/models/src/models/Subscriptions.ts"]
SubscriptionsRaw.findByRoomId["SubscriptionsRaw.findByRoomId"]
SubscriptionsRaw.findConnectedUsersExcept["SubscriptionsRaw.findConnectedUsersExcept"]
end
style SG11 fill:#2a2a2a,stroke:#444,color:#aaa
Spotlight.searchUsers --> readSecondaryPreferred
Spotlight.searchUsers --> Spotlight.mapOutsiders
Spotlight.searchUsers --> Spotlight.processLimitAndUsernames
Spotlight.searchUsers --> Spotlight._searchInsiderUsers
Spotlight.searchUsers --> Spotlight._searchConnectedUsers
Spotlight.searchUsers --> Spotlight._searchOutsiderUsers
Spotlight.searchUsers --> Spotlight._searchTeams
Spotlight.searchUsers --> ._Rocket.Chat_apps_meteor_app_authorization_server_index.ts
Spotlight.searchUsers --> hasAllPermissionAsync
Spotlight.searchUsers --> CachedSettings.get
Spotlight.searchUsers --> SubscriptionsRaw.findByRoomId
Spotlight._searchInsiderUsers --> Spotlight.processLimitAndUsernames
Spotlight._searchInsiderUsers --> CachedSettings.get
Spotlight._searchInsiderUsers --> trim
Spotlight._searchConnectedUsers --> Spotlight.processLimitAndUsernames
Spotlight._searchConnectedUsers --> CachedSettings.get
Spotlight._searchConnectedUsers --> trim
Spotlight._searchConnectedUsers --> SubscriptionsRaw.findConnectedUsersExcept
Spotlight._searchOutsiderUsers --> Spotlight.processLimitAndUsernames
Spotlight._searchOutsiderUsers --> CachedSettings.get
Spotlight._searchOutsiderUsers --> trim
Spotlight._searchTeams --> Spotlight.mapTeams
Spotlight._searchTeams --> CachedSettings.get
hasAllPermissionAsync --> Authorization.hasAllPermission
CachedSettings.get --> CachedSettings.get
SubscriptionsRaw.findByRoomId --> BaseRaw.find
trim --> makeString
trim --> defaultToWhiteSpace
Authorization.hasAllPermission --> Authorization.all
BaseRaw.find --> BaseRaw.doNotMixInclusionAndExclusionFields
BaseRaw.find --> BaseRaw.find
defaultToWhiteSpace --> makeString
BaseRaw.doNotMixInclusionAndExclusionFields --> BaseRaw.ensureDefaultFields
spotlightMethod --> Spotlight.searchUsers
spotlight --> spotlightMethod
action --> spotlightMethod
API.get.arg3 --> spotlightMethod
action_2 --> spotlightMethod
API.get.arg3_2 --> spotlightMethod
Fix with AI
A security vulnerability was found by Hacktron.
File: apps/meteor/app/api/server/v1/ai-search.ts
Severity: high
Vulnerability: Private Room and Private Team Membership Disclosure via Spotlight Search Authorization Bypass
Description:
An authorization bypass vulnerability exists in the Rocket.Chat spotlight search feature, allowing any registered user to disclose the membership list of any private room or private team on the server.
### Root Cause
In `Spotlight.searchUsers`, the variable `canListOutsiders` checks if the requester has the permissions `view-outside-room` and `view-d-room` (which are standard permissions granted to the default `user` role to allow starting direct messages with other users globally).
However, the variable `canListInsiders` is computed as:
`const canListInsiders = canListOutsiders || (rid && (await canAccessRoomAsync(room, { _id: userId })));`
Because `canListOutsiders` is true for standard users, `canListInsiders` is evaluated as `true` even if the user is not a member of the room and `canAccessRoomAsync` returns `false`.
If `canListInsiders` is true and a room ID (`rid`) is provided, the search logic queries for users belonging to that specific room (`__rooms: rid`) via `_searchInsiderUsers`. This allows any authenticated user to search for and retrieve the list of users inside any private room, bypassing the room access authorization check.
### Preconditions
- The attacker must have a registered account on the Rocket.Chat instance (possessing the default `user` role with `view-outside-room` and `view-d-room` permissions).
- The attacker must obtain or guess the target private room's ID (`rid`).
### Exploitation Path
1. The attacker registers a standard user account on the target Rocket.Chat instance.
2. The attacker identifies the `rid` of a target private room or private team.
3. The attacker calls the `spotlight` API endpoint (or invokes the DDP `spotlight` method) with parameters `rid=<target_rid>` and `query=<a>`.
4. The server returns matching users who are members of the private room.
5. By repeating this query with different search terms, the attacker can completely reconstruct the membership list of the private room.
### Security Impact
This vulnerability leads to a severe disclosure of sensitive information. Private rooms are designed to restrict visibility of their existence and membership to authorized members only. Leaking the identity of users in private channels (e.g., executive, legal, HR, or security-related channels) violates the core trust boundaries of the platform.
Proof of Concept:
An attacker can send the following HTTP request using their authentication token to retrieve users inside a target private room:
```bash
curl -H "X-Auth-Token: <your_auth_token>" \
-H "X-User-Id: <your_user_id>" \
"http://localhost:3000/api/v1/spotlight?query=a&rid=<private_room_id>&type=users"
```
The response will contain the list of users belonging to the private room whose names or usernames match the query "a".
Affected Code:
- In [spotlight.js:190-191](./Rocket.Chat/apps/meteor/server/lib/spotlight.js:190-191), `canListInsiders` is computed as `canListOutsiders || (rid && (await canAccessRoomAsync(room, { _id: userId })))`:
```javascript
const canListOutsiders = await hasAllPermissionAsync(userId, ['view-outside-room', 'view-d-room']);
const canListInsiders = canListOutsiders || (rid && (await canAccessRoomAsync(room, { _id: userId })));
```
- In [spotlight.js:255-258](./Rocket.Chat/apps/meteor/server/lib/spotlight.js:255-258), if `canListInsiders` is true and `rid` is provided, the search for room insiders is executed:
```javascript
if (canListInsiders && rid) {
// Search for insiders
if (await this._searchInsiderUsers(searchParams)) {
return users;
}
}
```
- In [spotlight.js:92-98](./Rocket.Chat/apps/meteor/server/lib/spotlight.js:92-98), `_searchInsiderUsers` queries the database using `insiderExtraQuery` (which contains `{ __rooms: rid }` for standard rooms):
```javascript
async _searchInsiderUsers({ rid, text, usernames, options, users, insiderExtraQuery, match = { startsWith: false, endsWith: false } }) {
// Get insiders first
if (rid) {
const searchFields = settings.get('Accounts_SearchFields').trim().split(',');
users.push(...(await Users.findByActiveUsersExcept(text, usernames, options, searchFields, insiderExtraQuery, match).toArray()));
```
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Member
There was a problem hiding this comment.
@Dnouv the issue looks real, but the severity feels overrated. An attacker would have to know RIDs of private rooms, which currently there's no easy way for them to get. The issue exists, but exploitability is hard due to the key piece here: the room ID.
Member
Author
There was a problem hiding this comment.
I do not think this is introduced by this PR. The AI Search endpoint does not expose room-scoped spotlight results: search.unified skips spotlight entirely when rid is present (rid || !includeSpotlight ? { users: [], rooms: [] } : spotlightMethod(...)). The reproduction targets the existing /api/v1/spotlight?rid=... / DDP spotlight behavior outside the AI Search changes. If still reproducible, it should be handled as a separate spotlight authorization fix rather than blocking this AI Search PR.
Comment on lines
+386
to
+415
| async function action() { | ||
| const { query, messages } = this.bodyParams; | ||
| let answer; | ||
| try { | ||
| answer = await AISearch.answer({ | ||
| query, | ||
| messages: messages.map(({ text, username, roomName, ts, score }) => ({ | ||
| text, | ||
| username, | ||
| roomName, | ||
| ts, | ||
| score, | ||
| })), | ||
| }); | ||
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : ''; | ||
| if (message.includes('error-ai-not-enabled')) { | ||
| throw new Meteor.Error('error-ai-not-enabled', 'AI Search is not enabled'); | ||
| } | ||
| if (message.includes('error-ai-provider-not-configured')) { | ||
| throw new Meteor.Error('error-ai-provider-not-configured', 'AI answer provider is not configured'); | ||
| } | ||
| if (message.includes('error-ai-provider-empty-response')) { | ||
| throw new Meteor.Error('error-ai-provider-empty-response', 'AI answer provider returned an empty response'); | ||
| } | ||
| throw new Meteor.Error('error-ai-provider-request-failed', 'AI answer provider request failed'); | ||
| } | ||
|
|
||
| return API.v1.success(answer); | ||
| }, |
There was a problem hiding this comment.
The Rocket.Chat API endpoint POST /api/v1/search.answer allows users to generate an AI-powered answer/summary based on a search query and a list of messages. However, the endpoint accepts the messages array directly from the client's request body (this.bodyParams) and passes it to the AI service (AISearch.answer) without any server-side validation.
The server does not verify:
- Whether the messages actually exist in the database.
- Whether the messages belong to the specified rooms or users.
- Whether the requesting user has the authorization/permissions to view these messages or rooms.
This lack of validation allows any authenticated user (with the default user role) to:
- Spoof Messages: An attacker can craft arbitrary messages pretending to be from any user (e.g., an administrator or executive) in any room (e.g., private channels). The AI will then generate a response summarizing these fake messages as if they were real, which can be used for sophisticated social engineering or spreading misinformation.
- Prompt Injection / LLM Abuse: An attacker can inject malicious instructions into the
queryormessagesfields to bypass the system prompt's constraints, or use the endpoint as a free proxy to query the underlying LLM for non-work-related tasks, leading to resource exhaustion and high API costs for the host organization.
Steps to Reproduce
An attacker can send the following authenticated HTTP request:
POST /api/v1/search.answer HTTP/1.1
Host: localhost:3000
X-Auth-Token: <user_token>
X-User-Id: <user_id>
Content-Type: application/json
{
"query": "What did the CEO say about my promotion?",
"messages": [
{
"text": "I have decided to promote the user and double their salary immediately.",
"username": "ceo_username",
"roomName": "executive-boardroom",
"ts": "2023-10-27T10:00:00.000Z",
"score": 0.99
}
]
}The server will process this request, send the spoofed message to the LLM, and return an AI-generated answer confirming the fake promotion:
{
"answer": "According to the search results, the CEO (@ceo_username) stated in the #executive-boardroom channel that they have decided to promote you and double your salary immediately.",
"provider": {
"name": "openai",
"model": "gpt-4"
},
"success": true
}Trace
graph TD
subgraph SG0 ["apps/meteor/app/api/server/helpers/getPaginationItems.ts"]
getPaginationItems["Calculates and validates pagination offset and count based on system settings."]
end
style SG0 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG1 ["apps/meteor/app/api/server/v1/ai-search.ts"]
parseCommaList["Parses a comma-separated string into an array, limited to a maximum number of values."]
parseQueryBoolean["Parses a query parameter into a boolean value."]
parseQueryDate["Parses a query parameter string into a Date object."]
getRoomMap["Fetches room details and maps them by ID for efficient lookups."]
action{{"Handles requests for generating AI-based search answers."}}
end
style SG1 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG2 ["apps/meteor/app/authorization/server/functions/canAccessRoom.ts"]
._Rocket.Chat_apps_meteor_app_authorization_server_functions_canAccessRoom.ts["Exports room access authorization helpers."]
end
style SG2 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG3 ["apps/meteor/app/authorization/server/functions/hasPermission.ts"]
hasAllPermissionAsync["hasAllPermissionAsync"]
hasPermissionAsync["hasPermissionAsync"]
end
style SG3 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG4 ["apps/meteor/app/authorization/server/index.ts"]
._Rocket.Chat_apps_meteor_app_authorization_server_index.ts["./Rocket.Chat/apps/meteor/app/authorization/server/index.ts"]
end
style SG4 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG5 ["apps/meteor/app/lib/server/lib/deprecationWarningLogger.ts"]
compareVersions["compareVersions"]
method["Logs deprecation warnings for methods."]
warn["warn"]
end
style SG5 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG6 ["apps/meteor/app/settings/server/CachedSettings.ts"]
CachedSettings.has["Checks if a setting exists in the cache."]
CachedSettings.getSetting["Retrieves the full setting object from the cache."]
CachedSettings.get["Retrieves a setting value from the cache."]
CachedSettings.set["Updates a setting in the cache and emits change events."]
end
style SG6 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG7 ["apps/meteor/app/settings/server/SettingsRegistry.ts"]
SettingsRegistry.add["Adds a new setting to the registry, validating and persisting it if necessary."]
SettingsRegistry.saveUpdatedSetting["Updates an existing setting in the database."]
end
style SG7 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG8 ["apps/meteor/app/settings/server/functions/getSettingDefaults.ts"]
getSettingDefaults["Applies default values and configurations to a setting object."]
end
style SG8 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG9 ["apps/meteor/app/settings/server/functions/validateSetting.ts"]
validateSetting["Validates the value of a setting based on its defined type."]
end
style SG9 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG10 ["apps/meteor/app/utils/server/lib/normalizeMessagesForUser.ts"]
filterStarred["Filters message starred status based on the current user ID."]
getNameOfUsername["Retrieves the display name for a given username from a map."]
normalizeMessagesForUser["Normalizes message data for a user, including name resolution and filtering."]
end
style SG10 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG11 ["apps/meteor/client/meteor/overrides/userAndUsers.ts"]
Meteor.userId["Overrides Meteor's default userId with a locally tracked version from the Zustand store."]
end
style SG11 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG12 ["apps/meteor/client/meteor/user.ts"]
watchUserId["watchUserId"]
end
style SG12 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG13 ["apps/meteor/client/meteor/watch.ts"]
watch["watch"]
end
style SG13 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG14 ["apps/meteor/lib/utils/stringUtils.ts"]
makeString["makeString"]
defaultToWhiteSpace["defaultToWhiteSpace"]
trim["trim"]
end
style SG14 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG15 ["apps/meteor/server/database/readSecondaryPreferred.ts"]
readSecondaryPreferred["Utility to determine MongoDB read preference for secondary preferred."]
end
style SG15 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG16 ["apps/meteor/server/lib/parseMessageSearchQuery.ts"]
MessageSearchQueryParser.constructor["MessageSearchQueryParser.constructor"]
MessageSearchQueryParser.consumeFrom["MessageSearchQueryParser.consumeFrom"]
MessageSearchQueryParser.consumeMention["MessageSearchQueryParser.consumeMention"]
MessageSearchQueryParser.consumeHasStar["MessageSearchQueryParser.consumeHasStar"]
MessageSearchQueryParser.consumeHasUrl["MessageSearchQueryParser.consumeHasUrl"]
MessageSearchQueryParser.consumeIsPinned["MessageSearchQueryParser.consumeIsPinned"]
MessageSearchQueryParser.consumeHasLocation["MessageSearchQueryParser.consumeHasLocation"]
MessageSearchQueryParser.consumeLabel["MessageSearchQueryParser.consumeLabel"]
MessageSearchQueryParser.consumeFileDescription["MessageSearchQueryParser.consumeFileDescription"]
MessageSearchQueryParser.consumeFileTitle["MessageSearchQueryParser.consumeFileTitle"]
MessageSearchQueryParser.consumeBefore["MessageSearchQueryParser.consumeBefore"]
MessageSearchQueryParser.consumeAfter["MessageSearchQueryParser.consumeAfter"]
MessageSearchQueryParser.consumeOn["MessageSearchQueryParser.consumeOn"]
MessageSearchQueryParser.consumeOrder["MessageSearchQueryParser.consumeOrder"]
MessageSearchQueryParser.consumeMessageText["MessageSearchQueryParser.consumeMessageText"]
MessageSearchQueryParser.parse["MessageSearchQueryParser.parse"]
parseMessageSearchQuery["Parses search query strings into MongoDB query objects."]
end
style SG16 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG17 ["apps/meteor/server/lib/rooms/roomCoordinator.ts"]
RoomCoordinatorServer.includeInRoomSearch["RoomCoordinatorServer.includeInRoomSearch"]
RoomCoordinatorServer.searchableRoomTypes["RoomCoordinatorServer.searchableRoomTypes"]
end
style SG17 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG18 ["apps/meteor/server/lib/spotlight.js"]
Spotlight.fetchRooms["Spotlight.fetchRooms"]
Spotlight.searchRooms["Searches for rooms based on a text query, applying authorization and visibility filters."]
Spotlight.mapOutsiders["Spotlight.mapOutsiders"]
Spotlight.processLimitAndUsernames["Spotlight.processLimitAndUsernames"]
Spotlight._searchInsiderUsers["Spotlight._searchInsiderUsers"]
Spotlight._searchConnectedUsers["Spotlight._searchConnectedUsers"]
Spotlight._searchOutsiderUsers["Spotlight._searchOutsiderUsers"]
Spotlight.mapTeams["Spotlight.mapTeams"]
Spotlight._searchTeams["Spotlight._searchTeams"]
Spotlight.searchUsers["Searches for users across the system, including insiders and outsiders, based on a text query and room context."]
end
style SG18 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG19 ["apps/meteor/server/methods/messageSearch.ts"]
messageSearch["messageSearch"]
end
style SG19 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG20 ["apps/meteor/server/publications/spotlight.ts"]
spotlightMethod["Orchestrates the spotlight search process by querying users and rooms."]
end
style SG20 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG21 ["apps/meteor/server/services/authorization/service.ts"]
Authorization.hasAllPermission["Authorization.hasAllPermission"]
Authorization.hasPermission["Authorization.hasPermission"]
Authorization.all["Authorization.all"]
end
style SG21 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG22 ["packages/apps/deno-runtime/AppObjectRegistry.ts"]
AppObjectRegistry.get["AppObjectRegistry.get"]
end
style SG22 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG23 ["packages/apps/deno-runtime/lib/logger.ts"]
Logger.error["Logs an error message."]
Logger.addEntry["Logger.addEntry"]
Logger.getStack["Logger.getStack"]
end
style SG23 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG24 ["packages/models/src/models/BaseRaw.ts"]
BaseRaw.doNotMixInclusionAndExclusionFields["BaseRaw.doNotMixInclusionAndExclusionFields"]
BaseRaw.find["BaseRaw.find"]
end
style SG24 fill:#2a2a2a,stroke:#444,color:#aaa
subgraph SG25 ["packages/models/src/models/Subscriptions.ts"]
SubscriptionsRaw.findByRoomId["SubscriptionsRaw.findByRoomId"]
SubscriptionsRaw.findByUserIdAndTypes["SubscriptionsRaw.findByUserIdAndTypes"]
SubscriptionsRaw.findConnectedUsersExcept["SubscriptionsRaw.findConnectedUsersExcept"]
end
style SG25 fill:#2a2a2a,stroke:#444,color:#aaa
action --> spotlightMethod
action --> CachedSettings.has
action --> CachedSettings.get
action --> normalizeMessagesForUser
action --> getPaginationItems
action --> messageSearch
action --> parseCommaList
action --> parseQueryBoolean
action --> parseQueryDate
action --> getRoomMap
spotlightMethod --> Spotlight.searchRooms
spotlightMethod --> Spotlight.searchUsers
CachedSettings.has --> CachedSettings.has
CachedSettings.get --> CachedSettings.get
normalizeMessagesForUser --> CachedSettings.get
normalizeMessagesForUser --> CachedSettings.set
normalizeMessagesForUser --> SettingsRegistry.add
normalizeMessagesForUser --> filterStarred
normalizeMessagesForUser --> getNameOfUsername
getPaginationItems --> CachedSettings.get
messageSearch --> readSecondaryPreferred
messageSearch --> parseMessageSearchQuery
messageSearch --> ._Rocket.Chat_apps_meteor_app_authorization_server_functions_canAccessRoom.ts
messageSearch --> CachedSettings.get
messageSearch --> Logger.error
messageSearch --> Meteor.userId
messageSearch --> method
messageSearch --> messageSearch
Spotlight.searchRooms --> Spotlight.fetchRooms
Spotlight.searchRooms --> RoomCoordinatorServer.searchableRoomTypes
Spotlight.searchRooms --> hasAllPermissionAsync
Spotlight.searchRooms --> CachedSettings.get
Spotlight.searchRooms --> trim
Spotlight.searchRooms --> SubscriptionsRaw.findByUserIdAndTypes
Spotlight.searchUsers --> readSecondaryPreferred
Spotlight.searchUsers --> Spotlight.mapOutsiders
Spotlight.searchUsers --> Spotlight.processLimitAndUsernames
Spotlight.searchUsers --> Spotlight._searchInsiderUsers
Spotlight.searchUsers --> Spotlight._searchConnectedUsers
Spotlight.searchUsers --> Spotlight._searchOutsiderUsers
Spotlight.searchUsers --> Spotlight._searchTeams
Spotlight.searchUsers --> ._Rocket.Chat_apps_meteor_app_authorization_server_index.ts
Spotlight.searchUsers --> hasAllPermissionAsync
Spotlight.searchUsers --> CachedSettings.get
Spotlight.searchUsers --> SubscriptionsRaw.findByRoomId
CachedSettings.set --> CachedSettings.has
CachedSettings.set --> CachedSettings.get
CachedSettings.set --> CachedSettings.set
SettingsRegistry.add --> CachedSettings.getSetting
SettingsRegistry.add --> CachedSettings.set
SettingsRegistry.add --> SettingsRegistry.saveUpdatedSetting
SettingsRegistry.add --> validateSetting
SettingsRegistry.add --> getSettingDefaults
getNameOfUsername --> CachedSettings.get
parseMessageSearchQuery --> MessageSearchQueryParser.constructor
parseMessageSearchQuery --> MessageSearchQueryParser.parse
Logger.error --> AppObjectRegistry.get
Logger.error --> Logger.addEntry
Logger.error --> Logger.getStack
Meteor.userId --> watchUserId
method --> compareVersions
method --> warn
Spotlight.fetchRooms --> hasPermissionAsync
Spotlight.fetchRooms --> CachedSettings.get
RoomCoordinatorServer.searchableRoomTypes --> RoomCoordinatorServer.includeInRoomSearch
hasAllPermissionAsync --> Authorization.hasAllPermission
trim --> makeString
trim --> defaultToWhiteSpace
SubscriptionsRaw.findByUserIdAndTypes --> BaseRaw.find
Spotlight._searchInsiderUsers --> Spotlight.processLimitAndUsernames
Spotlight._searchInsiderUsers --> CachedSettings.get
Spotlight._searchInsiderUsers --> trim
Spotlight._searchConnectedUsers --> Spotlight.processLimitAndUsernames
Spotlight._searchConnectedUsers --> CachedSettings.get
Spotlight._searchConnectedUsers --> trim
Spotlight._searchConnectedUsers --> SubscriptionsRaw.findConnectedUsersExcept
Spotlight._searchOutsiderUsers --> Spotlight.processLimitAndUsernames
Spotlight._searchOutsiderUsers --> CachedSettings.get
Spotlight._searchOutsiderUsers --> trim
Spotlight._searchTeams --> Spotlight.mapTeams
Spotlight._searchTeams --> CachedSettings.get
SubscriptionsRaw.findByRoomId --> BaseRaw.find
CachedSettings.getSetting --> CachedSettings.get
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeFrom
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeMention
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeHasStar
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeHasUrl
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeIsPinned
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeHasLocation
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeLabel
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeFileDescription
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeFileTitle
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeBefore
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeAfter
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeOn
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeOrder
MessageSearchQueryParser.parse --> MessageSearchQueryParser.consumeMessageText
watchUserId --> watch
warn --> compareVersions
warn --> warn
hasPermissionAsync --> Authorization.hasPermission
Authorization.hasAllPermission --> Authorization.all
defaultToWhiteSpace --> makeString
BaseRaw.find --> BaseRaw.doNotMixInclusionAndExclusionFields
BaseRaw.find --> BaseRaw.find
Authorization.hasPermission --> Authorization.all
Fix with AI
A security vulnerability was found by Hacktron.
File: apps/meteor/app/api/server/v1/ai-search.ts
Lines: 386-415
Severity: high
Vulnerability: Arbitrary Message Spoofing and Prompt Injection in AI Search Answer Endpoint (search.answer)
Description:
The Rocket.Chat API endpoint `POST /api/v1/search.answer` allows users to generate an AI-powered answer/summary based on a search query and a list of messages. However, the endpoint accepts the `messages` array directly from the client's request body (`this.bodyParams`) and passes it to the AI service (`AISearch.answer`) without any server-side validation.
The server does not verify:
1. Whether the messages actually exist in the database.
2. Whether the messages belong to the specified rooms or users.
3. Whether the requesting user has the authorization/permissions to view these messages or rooms.
This lack of validation allows any authenticated user (with the default `user` role) to:
- **Spoof Messages**: An attacker can craft arbitrary messages pretending to be from any user (e.g., an administrator or executive) in any room (e.g., private channels). The AI will then generate a response summarizing these fake messages as if they were real, which can be used for sophisticated social engineering or spreading misinformation.
- **Prompt Injection / LLM Abuse**: An attacker can inject malicious instructions into the `query` or `messages` fields to bypass the system prompt's constraints, or use the endpoint as a free proxy to query the underlying LLM for non-work-related tasks, leading to resource exhaustion and high API costs for the host organization.
Proof of Concept:
An attacker can send the following authenticated HTTP request:
```http
POST /api/v1/search.answer HTTP/1.1
Host: localhost:3000
X-Auth-Token: <user_token>
X-User-Id: <user_id>
Content-Type: application/json
{
"query": "What did the CEO say about my promotion?",
"messages": [
{
"text": "I have decided to promote the user and double their salary immediately.",
"username": "ceo_username",
"roomName": "executive-boardroom",
"ts": "2023-10-27T10:00:00.000Z",
"score": 0.99
}
]
}
```
The server will process this request, send the spoofed message to the LLM, and return an AI-generated answer confirming the fake promotion:
```json
{
"answer": "According to the search results, the CEO (@ceo_username) stated in the #executive-boardroom channel that they have decided to promote you and double your salary immediately.",
"provider": {
"name": "openai",
"model": "gpt-4"
},
"success": true
}
```
Affected Code:
- In [ai-search.ts](./Rocket.Chat/apps/meteor/app/api/server/v1/ai-search.ts#L371-L416):
```typescript
API.v1.post(
'search.answer',
{
authRequired: true,
body: isSearchAnswerProps,
rateLimiterOptions: {
numRequestsAllowed: 10,
intervalTimeInMS: 60000,
},
response: {
200: searchAnswerResponseSchema,
400: validateBadRequestErrorResponse,
401: validateUnauthorizedErrorResponse,
},
},
async function action() {
const { query, messages } = this.bodyParams;
let answer;
try {
answer = await AISearch.answer({
query,
messages: messages.map(({ text, username, roomName, ts, score }) => ({
text,
username,
roomName,
ts,
score,
})),
});
```
- In [llm.ts](./Rocket.Chat/packages/ai-search/src/llm.ts#L4-L27):
```typescript
export const buildSearchAnswerPrompt = (
query: string,
messages: SearchAnswerMessage[],
options: {
maxMessages: number;
maxTextLength: number;
},
): string =>
[
`User search query: ${query}`,
'Search results:',
...messages.slice(0, options.maxMessages).map((message, index) => {
const metadata = [
message.username && `from @${message.username}`,
message.roomName && `in #${message.roomName}`,
message.ts && `at ${message.ts}`,
typeof message.score === 'number' && `score ${Math.round(message.score * 100)}%`,
]
.filter(Boolean)
.join(', ');
return `${index + 1}. ${metadata ? `[${metadata}] ` : ''}${message.text.slice(0, options.maxTextLength)}`;
}),
'Answer using only the search results above. If the results do not contain enough information, say that clearly.',
].join('\n\n');
```
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Member
There was a problem hiding this comment.
@Dnouv looks like the end user would be the only user that would see the spoofed message, which means the damage may be lower than what the scan is stating. Still worth taking a look into, though.
Member
Author
There was a problem hiding this comment.
Addressed in f2717b7f37 and 5b6a3a79c7. search.answer no longer trusts client-provided message text/user/room metadata: the client sends source IDs plus scores, and the server rehydrates visible messages, filters them by the requester’s room subscriptions via Subscriptions.findByUserIdAndRoomIds, normalizes those messages for the requester, and only then sends verified sources to the LLM. If no supplied sources are visible and accessible, the endpoint rejects the request.
Comment on lines
+43
to
+63
| const getUserIdsByUsernames = async (usernames: string[]): Promise<string[] | undefined> => { | ||
| const requestedUsernames = usernames.map((username) => getUsernameLookupValues(username)).filter((values) => values.length); | ||
| if (!requestedUsernames.length) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const usernameLookupValues = [...new Set(requestedUsernames.flat())]; | ||
| const users = await Users.find( | ||
| { username: { $in: usernameLookupValues } }, | ||
| { | ||
| projection: { _id: 1, username: 1 }, | ||
| }, | ||
| ).toArray(); | ||
|
|
||
| const foundUsernames = new Set(users.map(({ username }) => username)); | ||
| if (requestedUsernames.some((values) => !values.some((username) => foundUsernames.has(username)))) { | ||
| return []; | ||
| } | ||
|
|
||
| return users.map(({ _id }) => _id); | ||
| }; |
There was a problem hiding this comment.
An information disclosure vulnerability exists in the message search functionality of Rocket.Chat. When an authenticated user performs a search filtering by sender usernames (fromUsernames or fromUsername), the server resolves the usernames to user IDs using Users.find without checking if the requesting user has permission to view those users.
If any of the requested usernames do not exist, getUserIdsByUsernames returns []. If all exist, it returns their IDs.
In messageSearch, if filterUserIds is empty, the search returns an empty result immediately ({ message: { docs: [] } }).
An attacker can exploit this behavior as a side-channel to determine the existence of any username on the server (including private, admin, or deactivated accounts). By searching for a message they sent themselves (e.g., "my_test_message") and filtering by fromUsernames: ['attacker_username', 'target_username']:
- If
target_usernameexists,filterUserIdsis not empty, and the search returns the attacker's message. - If
target_usernamedoes not exist,filterUserIdsis empty, and the search returns an empty result immediately.
Steps to Reproduce
An attacker posts a unique message "my_test_message" in any room.
They send a GET request to /api/v1/search.unified with parameters:
query=my_test_message&includeMessages=true&fromUsernames=attacker_username,secret_admin
- If the response contains
"my_test_message", thensecret_adminexists on the server. - If the response is empty, then
secret_admindoes NOT exist.
Fix with AI
A security vulnerability was found by Hacktron.
File: apps/meteor/server/methods/messageSearch.ts
Lines: 43-63
Severity: medium
Vulnerability: Username Existence Disclosure via Message Search Side-Channel
Description:
An information disclosure vulnerability exists in the message search functionality of Rocket.Chat. When an authenticated user performs a search filtering by sender usernames (`fromUsernames` or `fromUsername`), the server resolves the usernames to user IDs using `Users.find` without checking if the requesting user has permission to view those users.
If any of the requested usernames do not exist, `getUserIdsByUsernames` returns `[]`. If all exist, it returns their IDs.
In `messageSearch`, if `filterUserIds` is empty, the search returns an empty result immediately (`{ message: { docs: [] } }`).
An attacker can exploit this behavior as a side-channel to determine the existence of any username on the server (including private, admin, or deactivated accounts). By searching for a message they sent themselves (e.g., `"my_test_message"`) and filtering by `fromUsernames: ['attacker_username', 'target_username']`:
- If `target_username` exists, `filterUserIds` is not empty, and the search returns the attacker's message.
- If `target_username` does not exist, `filterUserIds` is empty, and the search returns an empty result immediately.
Proof of Concept:
An attacker posts a unique message `"my_test_message"` in any room.
They send a GET request to `/api/v1/search.unified` with parameters:
`query=my_test_message&includeMessages=true&fromUsernames=attacker_username,secret_admin`
- If the response contains `"my_test_message"`, then `secret_admin` exists on the server.
- If the response is empty, then `secret_admin` does NOT exist.
Affected Code:
- In [messageSearch.ts](./Rocket.Chat/apps/meteor/server/methods/messageSearch.ts):
```typescript
const getUserIdsByUsernames = async (usernames: string[]): Promise<string[] | undefined> => {
const requestedUsernames = usernames.map((username) => getUsernameLookupValues(username)).filter((values) => values.length);
if (!requestedUsernames.length) {
return undefined;
}
const usernameLookupValues = [...new Set(requestedUsernames.flat())];
const users = await Users.find(
{ username: { $in: usernameLookupValues } },
{
projection: { _id: 1, username: 1 },
},
).toArray();
const foundUsernames = new Set(users.map(({ username }) => username));
if (requestedUsernames.some((values) => !values.some((username) => foundUsernames.has(username)))) {
return [];
}
return users.map(({ _id }) => _id);
};
```
- In [messageSearch.ts](./Rocket.Chat/apps/meteor/server/methods/messageSearch.ts):
```typescript
const filterUserIds = await getUserIdsByUsernames([
...(filters?.fromUsernames || []),
...(filters?.fromUsername ? [filters.fromUsername] : []),
]);
if (filterUserIds) {
if (!filterUserIds.length) {
return {
message: {
docs: [],
},
};
}
query['u._id'] = filterUserIds.length === 1 ? filterUserIds[0] : { $in: filterUserIds };
}
```
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Member
There was a problem hiding this comment.
!fp There are several ways for an authenticated user to get existing users' usernames. It's not a vulnerability.
Member
Author
There was a problem hiding this comment.
Addressed in 5748017650. The username filter no longer treats one unresolved username in a multi-user filter as an all-empty result: getUserIdsByUsernames now returns the resolvable user IDs and messageSearch applies those IDs. That removes the reported attacker+target behavior flip based on whether the second username exists. The broader username-existence concern is already exposed through existing authenticated product surfaces, so this change keeps the AI Search path aligned without adding a new side channel.
Comment on lines
+65
to
+74
| const getRoomIdsByNames = async (roomNames: string[]): Promise<string[] | undefined> => { | ||
| const normalizedRoomNames = [...new Set(roomNames.filter(Boolean))]; | ||
| if (!normalizedRoomNames.length) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const rooms = await Promise.all(normalizedRoomNames.map((roomName) => Rooms.findOneByNameOrFname(roomName, { projection: { _id: 1 } }))); | ||
|
|
||
| return rooms.some((room) => !room) ? [] : rooms.map((room) => room?._id).filter((roomId): roomId is string => Boolean(roomId)); | ||
| }; |
There was a problem hiding this comment.
An information disclosure vulnerability exists in the message search functionality of Rocket.Chat. When an authenticated user performs a unified search or message search with a list of room names (roomNames), the server resolves the room names to room IDs using Rooms.findOneByNameOrFname without checking if the user has access to those rooms.
If any of the specified room names do not exist, getRoomIdsByNames returns an empty array []. If all room names exist, it returns their IDs.
In messageSearch, if the resolved filterRoomIds is empty, the search query is performed across all of the user's subscribedRoomIds. If filterRoomIds is not empty, the query is restricted to the intersection of subscribedRoomIds and filterRoomIds.
An attacker can exploit this behavior as a side-channel to determine the existence of any private room or private team on the server. By searching for a message known to exist in a room they are subscribed to (e.g., RoomB) and filtering by roomNames: ['RoomA', 'TargetPrivateRoom'] (where RoomA is another room they are subscribed to):
- If
TargetPrivateRoomexists, the search query is restricted toRoomA, and the message fromRoomBis not returned. - If
TargetPrivateRoomdoes not exist, the search query falls back to all subscribed rooms, and the message fromRoomBis returned.
Steps to Reproduce
An attacker is a member of RoomA and RoomB. They post a unique message "test_msg_in_b" in RoomB.
They send a GET request to /api/v1/search.unified with parameters:
query=test_msg_in_b&includeMessages=true&roomNames=RoomA,SecretPrivateRoom
- If the response contains the message
"test_msg_in_b", thenSecretPrivateRoomdoes NOT exist. - If the response is empty, then
SecretPrivateRoomexists.
Fix with AI
A security vulnerability was found by Hacktron.
File: apps/meteor/server/methods/messageSearch.ts
Lines: 65-74
Severity: medium
Vulnerability: Private Room and Private Team Existence Disclosure via Message Search Side-Channel
Description:
An information disclosure vulnerability exists in the message search functionality of Rocket.Chat. When an authenticated user performs a unified search or message search with a list of room names (`roomNames`), the server resolves the room names to room IDs using `Rooms.findOneByNameOrFname` without checking if the user has access to those rooms.
If any of the specified room names do not exist, `getRoomIdsByNames` returns an empty array `[]`. If all room names exist, it returns their IDs.
In `messageSearch`, if the resolved `filterRoomIds` is empty, the search query is performed across all of the user's `subscribedRoomIds`. If `filterRoomIds` is not empty, the query is restricted to the intersection of `subscribedRoomIds` and `filterRoomIds`.
An attacker can exploit this behavior as a side-channel to determine the existence of any private room or private team on the server. By searching for a message known to exist in a room they are subscribed to (e.g., `RoomB`) and filtering by `roomNames: ['RoomA', 'TargetPrivateRoom']` (where `RoomA` is another room they are subscribed to):
- If `TargetPrivateRoom` exists, the search query is restricted to `RoomA`, and the message from `RoomB` is not returned.
- If `TargetPrivateRoom` does not exist, the search query falls back to all subscribed rooms, and the message from `RoomB` is returned.
Proof of Concept:
An attacker is a member of `RoomA` and `RoomB`. They post a unique message `"test_msg_in_b"` in `RoomB`.
They send a GET request to `/api/v1/search.unified` with parameters:
`query=test_msg_in_b&includeMessages=true&roomNames=RoomA,SecretPrivateRoom`
- If the response contains the message `"test_msg_in_b"`, then `SecretPrivateRoom` does NOT exist.
- If the response is empty, then `SecretPrivateRoom` exists.
Affected Code:
- In [messageSearch.ts](./Rocket.Chat/apps/meteor/server/methods/messageSearch.ts):
```typescript
const getRoomIdsByNames = async (roomNames: string[]): Promise<string[] | undefined> => {
const normalizedRoomNames = [...new Set(roomNames.filter(Boolean))];
if (!normalizedRoomNames.length) {
return undefined;
}
const rooms = await Promise.all(normalizedRoomNames.map((roomName) => Rooms.findOneByNameOrFname(roomName, { projection: { _id: 1 } })));
return rooms.some((room) => !room) ? [] : rooms.map((room) => room?._id).filter((roomId): roomId is string => Boolean(roomId));
};
```
- In [messageSearch.ts](./Rocket.Chat/apps/meteor/server/methods/messageSearch.ts):
```typescript
const filterRoomIds = [...(filters?.rids || []), ...((await getRoomIdsByNames(filters?.roomNames || [])) || [])].filter(Boolean);
...
query.rid = {
$in: filterRoomIds.length ? subscribedRoomIds.filter((roomId) => filterRoomIds.includes(roomId)) : subscribedRoomIds,
};
```
Acceptance criteria:
- Acceptance is defined by the **actual reported behavior**, not by tests passing.
- Reproduce the issue, or narrow the exact code path that produces it, *before* changing code. State what you confirmed.
- Fix the underlying cause. Mitigations that paper over the reported behavior do not count as a fix.
- Add a regression test that fails on the unpatched code and passes on the fix. If a regression test is genuinely impractical (e.g. race condition, infra-level issue), say so and explain why.
- Existing tests passing is **not** the bar. Do not declare done on tests-pass theatre.
Only change what is necessary to fix this vulnerability. Do not refactor adjacent code or modify unrelated files.
Triage: Reply !fp <reason> (false positive), !valid (confirmed), or !accepted_risk <reason>. Any other reply is saved as a triage note.
Reason is optional but improves future scans — e.g. !fp internal endpoint, not user-facing.
Member
Author
There was a problem hiding this comment.
Addressed in 5748017650. Room-name filters now preserve the requested filter intent separately from resolved room IDs. If roomNames is supplied but none of the requested rooms resolve or intersect the user subscriptions, messageSearch uses an empty room scope instead of falling back to all subscribed rooms. That removes the reported fallback side channel for unresolved room names.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/meteor/app/api/server/v1/ai-search.ts (1)
404-417:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
search.answerstill trusts client-provided messages without authorization bindingLine 405 and Line 408-416 still pass user-supplied
messagesdirectly toAISearch.answerwithout verifying those messages exist and are visible to the caller. This keeps spoofed-context answer generation possible and should be fixed at the endpoint boundary (e.g., accept message IDs and resolve authorized messages server-side before LLM call).🤖 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/api/server/v1/ai-search.ts` around lines 404 - 417, The action() function currently accepts user-supplied messages from this.bodyParams and passes them directly to AISearch.answer without verifying the messages exist or that the caller has permission to access them. Instead of passing full message objects from the client, modify the endpoint to accept message IDs, then resolve those IDs server-side by fetching the actual messages from the database and verifying the current user has authorization to view them before passing the authorized messages to AISearch.answer.
🤖 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.
Duplicate comments:
In `@apps/meteor/app/api/server/v1/ai-search.ts`:
- Around line 404-417: The action() function currently accepts user-supplied
messages from this.bodyParams and passes them directly to AISearch.answer
without verifying the messages exist or that the caller has permission to access
them. Instead of passing full message objects from the client, modify the
endpoint to accept message IDs, then resolve those IDs server-side by fetching
the actual messages from the database and verifying the current user has
authorization to view them before passing the authorized messages to
AISearch.answer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a3e482fb-cda3-4ee3-8b8e-e868242d3395
📒 Files selected for processing (16)
apps/meteor/app/api/server/v1/ai-search.tsapps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsxapps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.tsapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/client/views/search/SearchPage.tsxapps/meteor/server/methods/messageSearch.tsapps/meteor/server/services/ai-search/service.tsapps/meteor/server/settings/ai.tsapps/meteor/tests/unit/server/services/ai-search/service.tests.tsdocs/intelligent-search-core-vs-apps-engine.mdpackages/ai-search/src/intelligentSearch.spec.tspackages/ai-search/src/intelligentSearch.tspackages/ai-search/src/llm.spec.tspackages/ai-search/src/llm.tspackages/i18n/src/locales/en.i18n.jsonpackages/rest-typings/src/v1/misc.ts
💤 Files with no reviewable changes (1)
- apps/meteor/server/settings/ai.ts
✅ Files skipped from review due to trivial changes (1)
- docs/intelligent-search-core-vs-apps-engine.md
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/meteor/client/navbar/NavBarSearch/NavBarSearchMessageRow.tsx
- packages/rest-typings/src/v1/misc.ts
- packages/ai-search/src/llm.spec.ts
- packages/ai-search/src/intelligentSearch.spec.ts
- apps/meteor/tests/unit/server/services/ai-search/service.tests.ts
- packages/ai-search/src/llm.ts
- apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts
- packages/ai-search/src/intelligentSearch.ts
- apps/meteor/client/views/search/SearchPage.tsx
- apps/meteor/server/services/ai-search/service.ts
- packages/i18n/src/locales/en.i18n.json
📜 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: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: Hacktron Security Check
- GitHub Check: CodeQL-Build
🧰 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/messageSearch.tsapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/app/api/server/v1/ai-search.ts
🧠 Learnings (7)
📚 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/messageSearch.tsapps/meteor/app/api/server/v1/ai-search.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/messageSearch.tsapps/meteor/app/api/server/v1/ai-search.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/messageSearch.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/messageSearch.tsapps/meteor/client/views/admin/aiCenter/AICenterRoute.tsxapps/meteor/app/api/server/v1/ai-search.ts
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.
Applied to files:
apps/meteor/client/views/admin/aiCenter/AICenterRoute.tsx
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/ai-search.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/ai-search.ts
🔇 Additional comments (3)
apps/meteor/server/methods/messageSearch.ts (1)
24-31: LGTM!Also applies to: 33-84, 148-149, 153-158, 161-174, 177-177
apps/meteor/app/api/server/v1/ai-search.ts (1)
214-221: LGTM!Also applies to: 251-251, 271-280
apps/meteor/client/views/admin/aiCenter/AICenterRoute.tsx (1)
55-66: LGTM!Also applies to: 73-73
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/api/server/v1/ai-search.ts`:
- Around line 235-270: The getSearchAnswerMessagesForUser function lacks
room-level authorization checks and only verifies message visibility, allowing
users to access AI summaries of content from rooms they cannot access. After
retrieving the room map using getRoomMap, add authorization validation by
calling canAccessRoomIdAsync for each unique room ID present in the messages
array, similar to the authorization pattern used in the messageSearch flow. If a
user lacks access to any room, throw an appropriate Meteor.Error before
proceeding with message normalization and answer generation.
🪄 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: bd7bb85b-e652-4d88-b2b1-b0984e99c5fc
📒 Files selected for processing (3)
apps/meteor/app/api/server/v1/ai-search.tsapps/meteor/client/views/search/SearchPage.tsxpackages/rest-typings/src/v1/misc.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/views/search/SearchPage.tsx
- packages/rest-typings/src/v1/misc.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). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🧰 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/app/api/server/v1/ai-search.ts
🧠 Learnings (5)
📚 Learning: 2026-02-23T17:53:06.802Z
Learnt from: ggazzo
Repo: RocketChat/Rocket.Chat PR: 35995
File: apps/meteor/app/api/server/v1/rooms.ts:1107-1112
Timestamp: 2026-02-23T17:53:06.802Z
Learning: During PR reviews that touch endpoint files under apps/meteor/app/api/server/v1, enforce strict scope: if a PR targets a specific endpoint (e.g., rooms.favorite), do not propose changes to unrelated endpoints (e.g., rooms.invite) unless maintainers explicitly request them. Focus feedback on the touched endpoint's behavior, API surface, and related tests; avoid broad cross-endpoint changes in the same PR unless requested.
Applied to files:
apps/meteor/app/api/server/v1/ai-search.ts
📚 Learning: 2026-02-24T19:09:01.522Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 38974
File: apps/meteor/app/api/server/v1/im.ts:220-221
Timestamp: 2026-02-24T19:09:01.522Z
Learning: In Rocket.Chat OpenAPI migration PRs for endpoints under apps/meteor/app/api/server/v1, avoid introducing logic changes. Only perform scope-tight changes that preserve behavior; style-only cleanups (e.g., removing inline comments) may be deferred to follow-ups to keep the migration PR focused.
Applied to files:
apps/meteor/app/api/server/v1/ai-search.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/app/api/server/v1/ai-search.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/app/api/server/v1/ai-search.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/app/api/server/v1/ai-search.ts
🔇 Additional comments (4)
apps/meteor/app/api/server/v1/ai-search.ts (4)
1-27: LGTM!
272-403: LGTM!
405-424: LGTM!
441-466: LGTM!
Contributor
There was a problem hiding this comment.
1 issue found across 16 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/server/settings/index.ts">
<violation number="1" location="apps/meteor/server/settings/index.ts:43">
P3: AI settings now include `AI_Thread_Summarization`, but the AI Center UI only routes to Intelligent_Search and AI_LLM_Provider. That leaves the new section unreachable from the feature preview entrypoint.</violation>
</file>
<file name="apps/meteor/app/api/server/v1/ai-search.ts">
<violation number="1" location="apps/meteor/app/api/server/v1/ai-search.ts:242">
P1: `search.answer` now trusts `findVisibleByIds` results without checking the caller can access each message room, enabling private message content to be used in generated answers.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
# Conflicts: # apps/meteor/client/navbar/NavBarSearch/NavBarSearch.tsx # apps/meteor/client/navbar/NavBarSearch/NavBarSearchListbox.tsx # apps/meteor/client/navbar/NavBarSearch/hooks/useSearchItems.ts
- i18n: localize NavBar filter suggestions (room/user/date) — remove hardcoded English; add Search_in_this_room and Search_messages_from_* - SearchPage: abort in-flight search.answer on query/source change and decouple the answer source set from pagination so "Show more" no longer retriggers a costly LLM generation (tie page size + answer slice to one constant) - search.answer: gate on AI Search license/feature availability before any DB work, and clamp client-supplied relevance scores to [0,1] - service: log the high-room-count unscoped-pipeline degraded path; move the default answer system prompt out of an indented template literal - a11y: wrap source results in a role="list" container - cleanup: drop the unused useFocusManager; document the deliberate helper duplication across the REST layer and AISearchService - tests: extend SearchPage source-row coverage (score clamping, non-link rendering) in the existing mockAppRoot style - docs: reframe completed work and clearly scope remaining items as future enhancements rather than open TODOs
Contributor
There was a problem hiding this comment.
1 issue found across 8 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/server/settings/index.ts">
<violation number="1" location="apps/meteor/server/settings/index.ts:43">
P3: AI settings now include `AI_Thread_Summarization`, but the AI Center UI only routes to Intelligent_Search and AI_LLM_Provider. That leaves the new section unreachable from the feature preview entrypoint.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
cardoso
requested changes
Jun 29, 2026
cardoso
left a comment
cardoso
left a comment
Member
There was a problem hiding this comment.
Initial review (backend only)
| @@ -0,0 +1,11 @@ | |||
| --- | |||
| '@rocket.chat/ai-search-service': minor | |||
Comment on lines
+1
to
+9
| export const trimTrailingSlashes = (url: string): string => { | ||
| let end = url.length; | ||
|
|
||
| while (end > 0 && url.charCodeAt(end - 1) === 47) { | ||
| end--; | ||
| } | ||
|
|
||
| return url.slice(0, end); | ||
| }; |
Member
There was a problem hiding this comment.
Is this file needed?
In service.ts:90 you use const normalizedBaseUrl = asString(baseUrl).replace(/\/+$/, '');. This separate function looks cryptic in comparison.
| "For formatting the answer, use markdown. For code snippets, use markdown code blocks with the appropriate language specified. Keep the answers as concise as possible, while still providing a complete answer to the user's question, and everything in a single column, without using tables or other formatting that may be hard to read in the Rocket.Chat client.", | ||
| ].join('\n'); | ||
|
|
||
| const aiServiceFetch: AIServiceFetch = (url, options) => fetch(url, options as Parameters<typeof fetch>[1]); |
Member
There was a problem hiding this comment.
Why have this AIServiceFetch type and cast the options param instead of using server-fetch directly?
Comment on lines
+394
to
+411
| async models(): Promise<AISearchModelOption[]> { | ||
| const [baseUrl, apiKey, selectedModel] = await Promise.all([ | ||
| Settings.get<string>('AI_LLM_OpenAI_Base_URL'), | ||
| Settings.get<string>('AI_LLM_OpenAI_API_Key'), | ||
| Settings.get<string>('AI_LLM_OpenAI_Model'), | ||
| ]); | ||
|
|
||
| return listOpenAICompatibleModels({ | ||
| provider: { | ||
| baseUrl: asString(baseUrl).replace(/\/+$/, ''), | ||
| apiKey: asString(apiKey), | ||
| }, | ||
| selectedModel: asString(selectedModel), | ||
| fetch: aiServiceFetch, | ||
| logger: SystemLogger, | ||
| }); | ||
| } | ||
| } |
Member
There was a problem hiding this comment.
In lines 109-130, you read the same settings:
private async getAnswerProviderConfig(): Promise<OpenAICompatibleProviderConfig | undefined> {
const [baseUrl, apiKey, model] = await Promise.all([
Settings.get<string>('AI_LLM_OpenAI_Base_URL'),
Settings.get<string>('AI_LLM_OpenAI_API_Key'),
Settings.get<string>('AI_LLM_OpenAI_Model'),
]);
const normalizedBaseUrl = asString(baseUrl).replace(/\/+$/, '');
const normalizedApiKey = asString(apiKey);
const normalizedModel = asString(model);
if (!normalizedBaseUrl || !normalizedApiKey || !normalizedModel) {
return undefined;
}
return {
name: 'OpenAI compatible',
baseUrl: normalizedBaseUrl,
apiKey: normalizedApiKey,
model: normalizedModel,
};
}
Maybe reuse this code?
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
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.
Proposed changes (including videos or screenshots)
Screen.Recording.2026-06-16.at.7.29.42.PM.mov
Issue(s)
Steps to test or reproduce
Further comments
AI-53
Summary by CodeRabbit