fix(search): escape regex in from and mention filters for exact username matching#39942
Open
sharanyamahajan wants to merge 5 commits intoRocketChat:developfrom
Open
Conversation
Added a new line to app.json to improve configuration consistency. Changes- 1. Updated app.json 2. Added the required configuration line Notes-This is a minor configuration update and does not affect application functionality.
chore: update app.json configuration
Removed unnecessary line breaks in the UserCard component to improve code readability and maintain formatting consistency. No functional changes were introduced.
|
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
Contributor
WalkthroughThis change fixes a message search bug where filters for usernames containing dots (e.g., Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 1
🧹 Nitpick comments (4)
apps/meteor/server/lib/parseMessageSearchQuery.ts (4)
58-58: Remove implementation comment.As per coding guidelines, avoid code comments in the implementation.
Suggested fix
- // ✅ FIXED private consumeMention(text: string) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` at line 58, Remove the implementation comment "// ✅ FIXED" from the parseMessageSearchQuery module so the function/implementation (parseMessageSearchQuery) contains no inline implementation comments; edit the source to delete that comment line and ensure no other implementation-only comments remain in the same function or surrounding scope.
243-245: Avoidanytype; use proper typing foroptionsparameter.Using
anyremoves type safety. The constructor already defines the expected shape. Extract the type or use the constructor's parameter type.Suggested fix
-export function parseMessageSearchQuery(text: string, options: any) { +export function parseMessageSearchQuery( + text: string, + options: { + user?: IUser; + offset?: number; + limit?: number; + forceRegex?: boolean; + }, +) { const parser = new MessageSearchQueryParser(options); return parser.parse(text); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` around lines 243 - 245, The parseMessageSearchQuery function currently types its options parameter as any; change it to the concrete type expected by MessageSearchQueryParser's constructor (use the parser's constructor parameter type or exported interface used by MessageSearchQueryParser) so callers get proper typing. Update the signature of parseMessageSearchQuery(text: string, options: /* use MessageSearchQueryParser constructor param type */) and any related declarations to import or reference that type from where MessageSearchQueryParser is defined, ensuring type compatibility with new MessageSearchQueryParser(options).
201-206: Addprivatemodifier for consistency.All other
consume*methods are markedprivate, butconsumeOrderis missing this modifier.Suggested fix
- consumeOrder(text: string) { + private consumeOrder(text: string) { return text.replace(/(?:order|sort):(asc|desc)/g, (_: string, direction: string) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` around lines 201 - 206, The method consumeOrder is missing the private visibility modifier while all other consume* methods are private; make consumeOrder a private method by adding the private modifier to its declaration (i.e., change the consumeOrder(...) signature to private consumeOrder(...)) so it matches the other consume* methods and class encapsulation.
165-165: Remove implementation comment.As per coding guidelines, avoid code comments in the implementation.
Suggested fix
- // ✅ FIXED BUG HERE private consumeAfter(text: string) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/server/lib/parseMessageSearchQuery.ts` at line 165, Remove the implementation comment "// ✅ FIXED BUG HERE" from apps/meteor/server/lib/parseMessageSearchQuery.ts so the codebase follows the guideline to avoid inline implementation comments; locate the comment in the parseMessageSearchQuery function (or top-level body of that module) and delete it, leaving only executable code and any necessary explanatory JSDoc or function-level comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Around line 208-216: The class stores a forceRegex flag in the constructor but
consumeMessageText always sets a $text search and ignores it; either remove
forceRegex or honor it: update consumeMessageText in ParseMessageSearchQuery (or
the class where forceRegex is defined) to check this.forceRegex (or the
constructor param name) and when true build a regex-based query (e.g., set
this.query.text or this.query.$or with RegExp on message text) instead of
setting this.query.$text and projection, otherwise retain the existing $text
branch; alternatively, if regex support is intentionally dropped, remove the
forceRegex property and its constructor parameter and any callers that pass
Message_AlwaysSearchRegExp.
---
Nitpick comments:
In `@apps/meteor/server/lib/parseMessageSearchQuery.ts`:
- Line 58: Remove the implementation comment "// ✅ FIXED" from the
parseMessageSearchQuery module so the function/implementation
(parseMessageSearchQuery) contains no inline implementation comments; edit the
source to delete that comment line and ensure no other implementation-only
comments remain in the same function or surrounding scope.
- Around line 243-245: The parseMessageSearchQuery function currently types its
options parameter as any; change it to the concrete type expected by
MessageSearchQueryParser's constructor (use the parser's constructor parameter
type or exported interface used by MessageSearchQueryParser) so callers get
proper typing. Update the signature of parseMessageSearchQuery(text: string,
options: /* use MessageSearchQueryParser constructor param type */) and any
related declarations to import or reference that type from where
MessageSearchQueryParser is defined, ensuring type compatibility with new
MessageSearchQueryParser(options).
- Around line 201-206: The method consumeOrder is missing the private visibility
modifier while all other consume* methods are private; make consumeOrder a
private method by adding the private modifier to its declaration (i.e., change
the consumeOrder(...) signature to private consumeOrder(...)) so it matches the
other consume* methods and class encapsulation.
- Line 165: Remove the implementation comment "// ✅ FIXED BUG HERE" from
apps/meteor/server/lib/parseMessageSearchQuery.ts so the codebase follows the
guideline to avoid inline implementation comments; locate the comment in the
parseMessageSearchQuery function (or top-level body of that module) and delete
it, leaving only executable code and any necessary explanatory JSDoc or
function-level comments.
🪄 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: 04784f7b-a95c-43c8-bba0-3bdcf10caa5c
📒 Files selected for processing (2)
apps/meteor/client/components/UserCard/UserCard.tsxapps/meteor/server/lib/parseMessageSearchQuery.ts
💤 Files with no reviewable changes (1)
- apps/meteor/client/components/UserCard/UserCard.tsx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38262
File: apps/meteor/client/lib/utils/normalizeMessagePreview/getMessagePreview.ts:24-33
Timestamp: 2026-01-19T18:17:46.433Z
Learning: In the Rocket.Chat repository, usernames (lastMessage.u.username) and display names (lastMessage.u.name) are sanitized/escaped centrally elsewhere in the system, so individual display functions like getMessagePreview do not need to escape these values before interpolating them into strings.
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:18.785Z
Learning: In Rocket.Chat PR reviews, maintain strict scope boundaries—when a PR is focused on a specific endpoint (e.g., rooms.favorite), avoid reviewing or suggesting changes to other endpoints that were incidentally refactored (e.g., rooms.invite) unless explicitly requested by maintainers.
📚 Learning: 2026-03-16T23:33:15.721Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: apps/meteor/app/api/server/v1/users.ts:862-869
Timestamp: 2026-03-16T23:33:15.721Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs (e.g., PR `#39676` for users.register in apps/meteor/app/api/server/v1/users.ts), calls to `this.parseJsonQuery()` inside migrated handlers are intentionally preserved without adding a corresponding `query` AJV schema to the route options. Adding query-param schemas for the `fields`/`sort`/`query` parameters consumed by `parseJsonQuery()` is a separate cross-cutting concern shared by many endpoints (e.g., users.create, users.update, users.list) and is explicitly out of scope for individual endpoint migration PRs. Do not flag the absence of a `query` schema for `parseJsonQuery()` usage as a violation of OpenAPI/AJV contract during migration reviews.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-03-03T11:11:48.541Z
Learnt from: ahmed-n-abdeltwab
Repo: RocketChat/Rocket.Chat PR: 39230
File: apps/meteor/app/api/server/v1/chat.ts:214-222
Timestamp: 2026-03-03T11:11:48.541Z
Learning: In apps/meteor/server/lib/moderation/reportMessage.ts, the reportMessage function validates that description is not empty or whitespace-only with `if (!description.trim())`. When migrating the chat.reportMessage endpoint to OpenAPI, adding minLength validation to the schema preserves this existing behavior.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.ts
📚 Learning: 2026-02-24T19:09:09.561Z
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:09.561Z
Learning: In RocketChat/Rocket.Chat OpenAPI migration PRs for apps/meteor/app/api/server/v1 endpoints, maintainers prefer to avoid any logic changes; style-only cleanups (like removing inline comments) may be deferred to follow-ups to keep scope tight.
Applied to files:
apps/meteor/server/lib/parseMessageSearchQuery.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/server/lib/parseMessageSearchQuery.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/lib/parseMessageSearchQuery.ts
🔇 Additional comments (3)
apps/meteor/server/lib/parseMessageSearchQuery.ts (3)
47-52: Regex escaping and anchoring correctly fixes the search filter issue.The use of
escapeRegExpcombined with^...$anchors properly ensures exact username matching, preventing regex metacharacters like.from acting as wildcards.
106-146: Good addition ofescapeRegExpto label and file filters.Applying
escapeRegExptoconsumeLabel,consumeFileDescription, andconsumeFileTitleprovides consistent handling of user input containing regex metacharacters across all filter types.
218-240: LGTM!The arrow function wrappers cleanly maintain
thiscontext and improve readability.
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
1 participant
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)
This PR fixes incorrect message search results when using
from:andmention:filters with usernames containing regex special characters (e.g., dots).Currently, usernames are directly inserted into MongoDB
$regexqueries without escaping. This causes special characters like.to act as wildcards, leading to unintended matches.Example:
from:rocket.catalso matchesrocketXcat,rocket_cat, etc.Changes made:
escapeRegExpbefore constructing regex^and$anchors to enforce exact username matchingfrom:andmention:filters inparseMessageSearchQuery.tsconsumeAfterwhere an incorrect variable (beforeDate) was referencedResult:
Issue(s)
Fixes #39927
Steps to test or reproduce
rocket.catrocketXcatSummary by CodeRabbit
Style
Refactor