fix: findCustomRoles/countCustomRoles miss roles with unset protected field

[parashuramGP]

Proposed changes

findCustomRoles and countCustomRoles in packages/models/src/models/Roles.ts currently filter roles with { protected: false }. MongoDB equality comparison against false only matches documents where the field is explicitly false — documents where protected is missing/undefined are excluded.

On Rocket.Chat instances upgraded from older versions, custom roles created years ago do not have a protected field stored at all. As a result, these legitimate custom roles are silently dropped from both queries, causing false negatives when listing or counting them.

Switching the filter to { protected: { $ne: true } } makes the query match both false and missing values, which matches the semantic intent ("all roles that are not protected") and restores correct behavior for legacy databases.

Issue(s)

Fixes #40798

(Also patches countCustomRoles, which has the same bug — not mentioned in the issue but the fix is identical and trivial.)

Steps to test or reproduce

1. Insert a custom role into the rocketchat_roles collection without a protected field:

   db.rocketchat_roles.insertOne({ _id: 'legacy-role', name: 'legacy-role', scope: 'Users' })

2. Call Roles.findCustomRoles().toArray() — before this PR the legacy role is missing; after the PR it appears.
3. Same for Roles.countCustomRoles().

Further comments

• Two-line behavioral change in a single file plus a changeset.
• No type change (Filter<IRole> already permits { $ne: T } operator expressions).
• No tests changed because the existing test suite for Roles doesn't cover this query shape; happy to add a unit test if reviewers prefer.
• I'll sign the CLA on the PR.

Summary by CodeRabbit

Bug Fixes

• Fixed custom role filtering to correctly identify custom roles when the protected attribute is either explicitly disabled or undefined, ensuring consistent behavior across instances.

[dionisio-bot]

Looks like this PR is not ready to merge, because of the following issues:

• This PR is missing the 'stat: QA assured' label
• This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

[changeset-bot]

🦋 Changeset detected

Latest commit: 478fe97

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@rocket.chat/models	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

Walkthrough

In RolesRaw, the protected field filter used by findCustomRoles and countCustomRoles is changed from { protected: false } to { protected: { $ne: true } }, so roles where the field is missing or undefined are included as custom roles. A changeset entry documents the fix.

Custom roles protected filter fix

Layer / File(s)	Summary
Update protected filter in findCustomRoles and countCustomRoles packages/models/src/models/Roles.ts, .changeset/fix-find-custom-roles-protected-filter.md	Both methods replace protected: false with protected: { $ne: true } so that roles without a protected field are counted and returned as custom roles. Changeset entry documents the behavioral fix.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main bug fix: addressing the issue where findCustomRoles/countCustomRoles miss roles with unset protected field.
Linked Issues check	✅ Passed	The PR implementation directly addresses issue #40798 by changing the protected filter from { protected: false } to { protected: { $ne: true } }, which matches both explicit false and missing/undefined fields as requested.
Out of Scope Changes check	✅ Passed	All changes are strictly scoped to fixing the protected-role filtering logic in findCustomRoles and countCustomRoles, with no extraneous modifications.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ 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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/models/src/models/Roles.ts (1)

141-152: 🧹 Nitpick | 🔵 Trivial

Add test coverage for the filter change.

Verification confirms that no existing tests reference findCustomRoles or countCustomRoles, despite these methods being used in production code (AppRoleBridge and statistics calculation). This bug fix affects a subtle MongoDB filter behavior—consider adding unit tests that verify both methods correctly handle roles with missing protected fields to prevent regressions.

🤖 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/models/src/models/Roles.ts` around lines 141 - 152, Add unit tests
for the findCustomRoles and countCustomRoles methods in the Roles class to
verify they correctly filter out protected roles and handle roles with missing
protected fields. Create test cases that ensure both methods return only
non-protected custom roles (where protected is either explicitly false or
missing/undefined) to prevent regressions in this MongoDB filter behavior that
is currently used in production code like AppRoleBridge.

🤖 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.

Nitpick comments:
In `@packages/models/src/models/Roles.ts`:
- Around line 141-152: Add unit tests for the findCustomRoles and
countCustomRoles methods in the Roles class to verify they correctly filter out
protected roles and handle roles with missing protected fields. Create test
cases that ensure both methods return only non-protected custom roles (where
protected is either explicitly false or missing/undefined) to prevent
regressions in this MongoDB filter behavior that is currently used in production
code like AppRoleBridge.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 38004786-2147-4dd9-a75e-e831a89da521

📥 Commits

Reviewing files that changed from the base of the PR and between 7b54fb7 and 478fe97.

📒 Files selected for processing (2)

• .changeset/fix-find-custom-roles-protected-filter.md
• packages/models/src/models/Roles.ts

📜 Review details

⏰ Context from checks skipped due to timeout. (1)

• GitHub Check: cubic · AI code reviewer

🧰 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:

• packages/models/src/models/Roles.ts

🧠 Learnings (4)

📚 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/fix-find-custom-roles-protected-filter.md

📚 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:

• packages/models/src/models/Roles.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:

• packages/models/src/models/Roles.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:

• packages/models/src/models/Roles.ts

🔇 Additional comments (2)

packages/models/src/models/Roles.ts (1)

141-145: LGTM!

Also applies to: 147-152

.changeset/fix-find-custom-roles-protected-filter.md (1)

1-6: LGTM!

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

[Shevilll]

Thanks for the clear writeup and repro steps. Switching to { protected: { $ne: true } } is the right semantic for "all non-protected roles," including legacy docs where the field was never stored.

Two notes:

1. Heads up that #41031 makes the identical two-line change. This PR is the more tightly scoped of the two (query fix + changeset + linked issue), so it seems like the better one to land.
2. Since you offered — a small unit test would lock this in: insert a role without protected and assert it shows up in findCustomRoles() / is counted by countCustomRoles(). The existing suite doesn't exercise this query shape.

Looks correct to me otherwise.