feat(e2ee): passphrase requirements #37327
Conversation
|
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 612a1a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 42 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughExtracts E2EE passphrase UI into two components (ChangePassphrase, ResetPassphrase), enforces a 30+ char strong-passphrase policy with validation, adds typed password-policy scaffolding and a usePasswordPolicy hook, moves password verification UI into PasswordVerifierList/PasswordVerifierItem, refactors verification hooks, and updates an E2EE test to generate a 30‑char structured password. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ChangePassphrase
participant PasswordVerifierList
participant E2EE_API
Note right of PasswordVerifierList `#D7F9E0`: validations from usePasswordPolicy
User->>ChangePassphrase: Type new passphrase
ChangePassphrase->>PasswordVerifierList: Request validations
PasswordVerifierList-->>ChangePassphrase: Return validations
alt all validations valid
User->>ChangePassphrase: Submit
ChangePassphrase->>E2EE_API: e2e.changePassword(mutation)
E2EE_API-->>ChangePassphrase: Success
ChangePassphrase->>User: Success toast + reset form
else validations fail
ChangePassphrase->>User: Show requirement failures
end
sequenceDiagram
participant User
participant EndToEnd
participant ChangePassphrase
participant ResetPassphrase
participant E2EE_API
User->>EndToEnd: Open E2E settings
EndToEnd->>ChangePassphrase: Render
EndToEnd->>ResetPassphrase: Render
alt Change passphrase flow
User->>ChangePassphrase: Input & submit
ChangePassphrase->>E2EE_API: e2e.changePassword
E2EE_API-->>ChangePassphrase: Result
ChangePassphrase-->>User: Feedback
else Reset passphrase flow
User->>ResetPassphrase: Click reset
ResetPassphrase->>E2EE_API: trigger reset mutation
ResetPassphrase-->>User: Feedback
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25–35 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-10-16T21:09:51.816ZApplied to files:
📚 Learning: 2025-10-07T15:08:37.419ZApplied to files:
📚 Learning: 2025-10-07T15:08:37.419ZApplied to files:
⏰ 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). (3)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #37327 +/- ##
===========================================
+ Coverage 66.94% 67.02% +0.08%
===========================================
Files 3415 3418 +3
Lines 117705 117751 +46
Branches 21515 21533 +18
===========================================
+ Hits 78801 78928 +127
+ Misses 36227 36141 -86
- Partials 2677 2682 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/meteor/client/views/account/security/ResetPassphrase.tsx (1)
17-19: Consider adding loading state and disabling the button during mutation.The button can be clicked multiple times while the mutation is in progress, potentially triggering multiple concurrent reset operations.
Apply this diff to disable the button during mutation and show loading state:
- <Button onClick={() => resetE2EPassword.mutate()} data-qa-type='e2e-encryption-reset-key-button'> + <Button + onClick={() => resetE2EPassword.mutate()} + disabled={resetE2EPassword.isPending} + loading={resetE2EPassword.isPending} + data-qa-type='e2e-encryption-reset-key-button' + > {t('Reset_E2EE_password')} </Button>packages/ui-contexts/src/hooks/useVerifyPassword.ts (1)
18-29: Memoize the policy options to avoid unnecessary re-instantiationBecause we build a fresh object literal on every render,
usePasswordPolicyends up creating a brand-newPasswordPolicyinstance each time settings or the password param rerender. Wrapping the options inuseMemokeeps the policy stable and saves some churn, especially when this hook is called in high-frequency inputs.-const validate = usePasswordPolicy({ - enabled, - minLength, - maxLength, - forbidRepeatingCharacters, - forbidRepeatingCharactersCount, - mustContainAtLeastOneLowercase, - mustContainAtLeastOneUppercase, - mustContainAtLeastOneNumber, - mustContainAtLeastOneSpecialCharacter, - throwError: false, -}); +const options = useMemo( + () => ({ + enabled, + minLength, + maxLength, + forbidRepeatingCharacters, + forbidRepeatingCharactersCount, + mustContainAtLeastOneLowercase, + mustContainAtLeastOneUppercase, + mustContainAtLeastOneNumber, + mustContainAtLeastOneSpecialCharacter, + throwError: false, + }), + [ + enabled, + minLength, + maxLength, + forbidRepeatingCharacters, + forbidRepeatingCharactersCount, + mustContainAtLeastOneLowercase, + mustContainAtLeastOneUppercase, + mustContainAtLeastOneNumber, + mustContainAtLeastOneSpecialCharacter, + ], +); + +const validate = usePasswordPolicy(options);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
apps/meteor/client/views/account/security/ChangePassphrase.tsx(1 hunks)apps/meteor/client/views/account/security/EndToEnd.tsx(1 hunks)apps/meteor/client/views/account/security/ResetPassphrase.tsx(1 hunks)apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts(1 hunks)packages/password-policies/src/PasswordPolicy.ts(3 hunks)packages/password-policies/src/index.ts(1 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifier.tsx(1 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx(1 hunks)packages/ui-client/src/components/index.ts(1 hunks)packages/ui-contexts/src/hooks/usePasswordPolicy.ts(1 hunks)packages/ui-contexts/src/hooks/useVerifyPassword.ts(2 hunks)packages/ui-contexts/src/index.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/meteor/tests/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.spec.ts: All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Use descriptive test names that clearly communicate expected behavior
Use test.beforeAll() and test.afterAll() for setup and teardown
Use test.step() to organize complex test scenarios
Group related tests in the same file
Utilize Playwright fixtures (test, page, expect) consistently
Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Maintain test isolation between test cases
Ensure a clean state for each test execution
Ensure tests run reliably in parallel without shared state conflicts
Files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx}: Write concise, technical TypeScript/JavaScript with accurate typing
Follow DRY by extracting reusable logic into helper functions or page objects
Avoid code comments in the implementation
Files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
apps/meteor/tests/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
apps/meteor/tests/e2e/**/*.{ts,tsx}: Avoid using page.locator(); prefer semantic locators like page.getByRole, page.getByLabel, page.getByText, and page.getByTitle
Store commonly used locators in variables/constants for reuse
Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Implement proper wait strategies for dynamic content
Follow the Page Object Model pattern consistently
Files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
🧠 Learnings (12)
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsxapps/meteor/client/views/account/security/EndToEnd.tsxapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsxapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsxapps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (test, page, expect) consistently
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure a clean state for each test execution
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All Playwright test files must be located under apps/meteor/tests/e2e/ and use the .spec.ts extension (e.g., login.spec.ts)
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use test.step() to organize complex test scenarios
Applied to files:
apps/meteor/tests/e2e/e2e-encryption/e2ee-passphrase-management.spec.ts
🧬 Code graph analysis (7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (3)
apps/meteor/client/views/room/hooks/useE2EEState.ts (1)
useE2EEState(9-9)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(11-24)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(851-851)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
packages/ui-client/src/components/index.ts (1)
PasswordVerifierList(6-6)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifier.tsx (2)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
PasswordVerifierList(19-50)packages/ui-client/src/components/index.ts (1)
PasswordVerifierList(6-6)
apps/meteor/client/views/account/security/ResetPassphrase.tsx (1)
apps/meteor/client/views/hooks/useResetE2EPasswordMutation.ts (1)
useResetE2EPasswordMutation(6-24)
packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
packages/ui-contexts/src/index.ts (1)
usePasswordPolicy(88-88)
apps/meteor/client/views/account/security/EndToEnd.tsx (2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
ChangePassphrase(46-187)apps/meteor/client/views/account/security/ResetPassphrase.tsx (1)
ResetPassphrase(6-22)
packages/ui-contexts/src/hooks/useVerifyPassword.ts (1)
packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(11-24)
🪛 ast-grep (0.39.6)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[warning] 93-93: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[error] 94-94: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (7)
packages/password-policies/src/PasswordPolicy.ts (1)
3-25: LGTM! Clean type extraction.The introduction of
PasswordPolicyOptionsandPasswordPolicyValidationas exported types formalizes the previously inline type definitions, improving type safety and reusability across the codebase.apps/meteor/client/views/account/security/EndToEnd.tsx (1)
1-17: LGTM! Clean refactor.The extraction of inline form logic into dedicated
ChangePassphraseandResetPassphrasecomponents improves code organization and maintainability. The type signature update toComponentPropsWithoutRef<typeof Box>is appropriate.packages/ui-contexts/src/index.ts (1)
88-88: LGTM!The addition of the
usePasswordPolicyexport is consistent with other hook exports in this module.packages/ui-client/src/components/index.ts (1)
6-6: LGTM!The addition of the
PasswordVerifierListexport is consistent with other component exports in this module.packages/password-policies/src/index.ts (1)
1-1: LGTM!Re-exporting
PasswordPolicyOptionsandPasswordPolicyValidationfrom the package entry point enables consumers to import these types directly from@rocket.chat/password-policies, which is good API design.packages/ui-client/src/components/PasswordVerifier/PasswordVerifier.tsx (1)
11-15: LGTM! Clean separation of concerns.The refactor properly separates validation computation (via
useVerifyPassword) from presentation logic (delegated toPasswordVerifierList), improving code maintainability.packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
19-50: LGTM! Good accessibility implementation.The component properly handles empty validations and implements good accessibility practices with ARIA attributes (
aria-labelledby,aria-invalid,role='list').
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
68-81: Confirmation validator still skips re-run on valid passphrase editsEditing the primary passphrase to another policy-compliant value leaves
valid === true, so this effect never re-runs and the confirmation field keeps its stale value.isValidstaystrue, the “Save changes” button remains enabled, and only on submit do we surface the mismatch. Please make the effect depend onpassphrase(or otherwise re-trigger confirmation validation on any primary passphrase change) so mismatches are caught immediately.Apply this diff:
- useEffect(() => { + useEffect(() => { if (!valid) { resetField('confirmationPassphrase'); return; } if (confirmationPassphrase) { const validateConfirmation = async () => { await trigger('confirmationPassphrase'); }; void validateConfirmation(); } - }, [valid, confirmationPassphrase, resetField, trigger]); + }, [passphrase, valid, confirmationPassphrase, resetField, trigger]);
🧹 Nitpick comments (2)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (2)
27-28: Consider adding a fallback for translation keys.The translation key is constructed dynamically from
props.name, but no fallback value is provided tot(). If the translation key doesn't exist, users might see the raw key (e.g., "minLength-label") displayed in the UI.Consider providing a fallback:
- const requirementText = t(name, 'limit' in props ? { limit: props.limit } : undefined); + const requirementText = t(name, { + ...'limit' in props ? { limit: props.limit } : {}, + defaultValue: props.name + });Alternatively, ensure all translation keys are present at build time through type-checking or validation.
38-41: Remove redundantaria-hidden='false'attributes.Setting
aria-hidden='false'is unnecessary since elements are not hidden by default. The proper aria labels (aria-labelledbyandaria-label) are already in place.Apply this diff:
role='listitem' - aria-hidden='false' aria-labelledby={`${id}-icon ${id}-text`} > - <Icon id={`${id}-icon`} aria-label={t(icon.label)} aria-hidden='false' name={icon.name} color={icon.color} size='x16' mie={4} /> + <Icon id={`${id}-icon`} aria-label={t(icon.label)} name={icon.name} color={icon.color} size='x16' mie={4} /> <span id={`${id}-text`}>{requirementText}</span>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/meteor/client/views/account/security/ChangePassphrase.tsx(1 hunks)packages/password-policies/src/PasswordPolicy.ts(3 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifier.tsx(1 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx(1 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx(1 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifiers.spec.tsx(3 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
packages/ui-client/src/components/PasswordVerifier/PasswordVerifiers.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use expect matchers (toEqual, toContain, toBeTruthy, toHaveLength, etc.) instead of assert statements
Applied to files:
packages/ui-client/src/components/PasswordVerifier/PasswordVerifiers.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (e.g., toBeVisible, toHaveText)
Applied to files:
packages/ui-client/src/components/PasswordVerifier/PasswordVerifiers.spec.tsx
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
packages/ui-client/src/components/PasswordVerifier/PasswordVerifiers.spec.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx} : Use page.waitFor() with specific conditions and avoid hardcoded timeouts
Applied to files:
packages/ui-client/src/components/PasswordVerifier/PasswordVerifiers.spec.tsx
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
🧬 Code graph analysis (5)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifier.tsx (2)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (2)
PasswordVerifierListProps(8-12)PasswordVerifierList(14-39)packages/ui-contexts/src/hooks/useVerifyPassword.ts (1)
useVerifyPassword(6-31)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (2)
packages/password-policies/src/PasswordPolicy.ts (1)
PasswordPolicyValidation(35-39)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
PasswordVerifierItem(23-45)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
packages/password-policies/src/PasswordPolicy.ts (1)
PasswordPolicyValidation(35-39)
packages/password-policies/src/PasswordPolicy.ts (2)
packages/password-policies/src/index.ts (2)
PasswordPolicyOptions(1-1)PasswordPolicyValidation(1-1)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
PasswordPolicyValidation(4-4)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (5)
apps/meteor/client/views/room/hooks/useE2EEState.ts (1)
useE2EEState(9-9)packages/password-policies/src/PasswordPolicy.ts (2)
validate(179-247)error(102-115)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(15-28)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(851-851)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
PasswordVerifierList(14-39)
🪛 ast-grep (0.39.7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[warning] 101-101: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[error] 102-102: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
2-2: ****The import of
PasswordPolicyValidationfrom@rocket.chat/ui-contextsis correct. The type is properly re-exported through the ui-contexts package: it originates inpackages/password-policies/src/PasswordPolicy.ts, is re-exported frompackages/password-policies/src/index.ts, then throughpackages/ui-contexts/src/hooks/usePasswordPolicy.ts, and finally exposed atpackages/ui-contexts/src/index.ts(line 88). Using@rocket.chat/ui-contextsas the import source follows the intended public API boundary—no changes are needed.Likely an incorrect or invalid review comment.
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/password-policies/src/PasswordPolicy.ts (1)
249-249: Avoid widening the return type — use the specificPasswordPolicyParametersEntryunion.The explicit generic
<[name: string, params?: Record<string, number | boolean>]>permits any string as a policy name and any object as params, which discards the type safety provided byPasswordPolicyParametersEntry. Consumers lose IDE autocomplete for policy names and cannot narrow types to specific policies.Apply this diff to use the default (or explicit specific) type parameter:
- getPasswordPolicy(): PasswordPolicyType<[name: string, params?: Record<string, number | boolean>]> { + getPasswordPolicy(): PasswordPolicyType {Alternatively, if you prefer being explicit:
- getPasswordPolicy(): PasswordPolicyType<[name: string, params?: Record<string, number | boolean>]> { + getPasswordPolicy(): PasswordPolicyType<PasswordPolicyParametersEntry> {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/password-policies/src/PasswordPolicy.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
packages/password-policies/src/PasswordPolicy.ts (1)
3-40: Well-structured type definitions.The type system correctly uses mapped types, conditional types, and template literal types to create a strongly-typed password policy API. The discriminated union in
PasswordPolicyParametersEntry(lines 17-21) appropriately handles the difference between number-based policies (which include a params object) and boolean-based policies (which don't).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
38-38: Remove redundantaria-hidden='false'attributes.Elements are not aria-hidden by default, so explicitly setting
aria-hidden='false'is unnecessary. You can safely remove these attributes without affecting accessibility.- aria-hidden='false'Also applies to: 41-41
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
🧬 Code graph analysis (1)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
packages/password-policies/src/PasswordPolicy.ts (1)
PasswordPolicyValidation(35-39)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (1)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
6-17: Previous type constraint issue resolved.The incorrect
satisfies Record<string, IconProps>constraint has been properly removed. Theas constassertion is now appropriate for the variants object structure.
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
125-125: Make aria-describedby conditional to reference only rendered elements.The
aria-describedbyattribute includes${passphraseId}-hintunconditionally, but the hint element (line 132) is only rendered when!keysExist. Referencing non-existent IDs can confuse screen readers.As per coding guidelines
Apply this diff to conditionally include the hint ID:- aria-describedby={`${e2ePasswordExplanationId} ${passphraseId}-hint ${passphraseId}-error`} + aria-describedby={`${e2ePasswordExplanationId}${!keysExist ? ` ${passphraseId}-hint` : ''}${errors.passphrase ? ` ${passphraseId}-error` : ''}`}
🧹 Nitpick comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
76-79: Simplify the async invocation.The async wrapper function can be eliminated by directly using
void trigger(...).Apply this diff:
- if (confirmationPassphrase) { - const validateConfirmation = async () => { - await trigger('confirmationPassphrase'); - }; - void validateConfirmation(); - } + if (confirmationPassphrase) { + void trigger('confirmationPassphrase'); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
🧬 Code graph analysis (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (5)
apps/meteor/client/views/room/hooks/useE2EEState.ts (1)
useE2EEState(9-9)packages/password-policies/src/PasswordPolicy.ts (2)
validate(179-247)error(102-115)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(15-28)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(851-851)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
PasswordVerifierList(14-39)
🪛 ast-grep (0.39.7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[warning] 101-101: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[error] 102-102: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (6)
1-11: LGTM!All imports are properly organized and utilized in the component.
13-21: LGTM!The strong passphrase policy (30-character minimum with mixed character classes) aligns with E2EE security requirements. The frozen object prevents accidental mutations.
23-31: LGTM!Both hooks provide clean abstractions for E2EE state checking and passphrase validation.
33-44: LGTM!The mutation setup and default values are correctly configured.
98-103: LGTM!The usage of
dangerouslySetInnerHTMLwithDOMPurify.sanitizeis the correct approach for safely rendering HTML content. The static analysis warning can be safely ignored here.
150-191: LGTM!The confirmation field has proper conditional rendering and accessibility attributes. The save button's disabled logic correctly validates all required conditions.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
155-158: Verify error handling for async operation.A previous review flagged that
e2e.decodePrivateKeyFlow()can throw errors and recommended wrapping it in a try-catch block. The comment was marked as addressed in commit 2297bea, but the current code still shows no error handling. Please verify this concern has been properly addressed.#!/bin/bash # Verify if decodePrivateKeyFlow has error handling or if it's wrapped elsewhere rg -n -A10 'decodePrivateKeyFlow\(\)' --type=ts --type=tsx
🧹 Nitpick comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
108-208: Consider using a semantic form element.While the current implementation works, wrapping the fields in a
<form>element withonSubmit={handleSubmit(handleSave)}would provide better semantics, native keyboard submission (Enter key), and improved accessibility. As per the previous review comment.Example refactor:
- <Box mbs={36} w='full'> + <Box is='form' mbs={36} w='full' onSubmit={handleSubmit(handleSave)}> <Box is='h3' fontScale='h4' mbe={12}> {t('Change_E2EE_password')} </Box> <FieldGroup w='full'> {/* ... fields ... */} </FieldGroup> <Button primary + type='submit' disabled={!(keysExist && valid && isValid)} - onClick={handleSubmit(handleSave)} mbs={12} data-qa-type='e2e-encryption-save-password-button' > {t('Save_changes')} </Button> </Box>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/meteor/client/views/account/security/ChangePassphrase.tsx(1 hunks)apps/meteor/client/views/account/security/EndToEnd.tsx(1 hunks)apps/meteor/client/views/account/security/ResetPassphrase.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/meteor/client/views/account/security/ResetPassphrase.tsx
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Avoid code comments in the implementation
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Write concise, technical TypeScript/JavaScript with accurate typing
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use descriptive test names that clearly communicate expected behavior
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsx
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsxapps/meteor/client/views/account/security/ChangePassphrase.tsx
📚 Learning: 2025-09-16T22:08:51.490Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-09-16T22:08:51.490Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,tsx,js,jsx} : Follow DRY by extracting reusable logic into helper functions or page objects
Applied to files:
apps/meteor/client/views/account/security/EndToEnd.tsx
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/account/security/EndToEnd.tsx (2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
ChangePassphrase(46-211)apps/meteor/client/views/account/security/ResetPassphrase.tsx (1)
ResetPassphrase(6-22)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (5)
apps/meteor/client/views/room/hooks/useE2EEState.ts (1)
useE2EEState(9-9)packages/password-policies/src/PasswordPolicy.ts (2)
validate(179-247)error(102-115)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(15-28)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(851-851)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
PasswordVerifierList(14-39)
🪛 ast-grep (0.39.7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[warning] 105-105: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[error] 106-106: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (7)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (6)
1-11: Imports look good.All dependencies are appropriate for the component's functionality: form handling, password policy validation, E2EE integration, and accessibility.
13-21: Strong E2EE passphrase policy implemented correctly.The 30-character minimum with character class requirements provides robust protection for E2EE keys.
Object.freezeensures the policy remains immutable.
23-44: Clean hook abstractions.The custom hooks (
useKeysExist,useValidatePassphrase,useChangeE2EPasswordMutation) provide good separation of concerns and will be easy to test independently.
59-85: Confirmation revalidation properly implemented.The effect now correctly revalidates the confirmation field when the primary passphrase changes to a valid value, addressing the previous review feedback. The inclusion of
triggerensures immediate validation feedback.
90-98: Save handler looks good.The mutation and toast handling is clean. As confirmed by the author, the error formatting is handled automatically by the
dispatchToastMessageinfrastructure.
102-107: Sanitized HTML rendering is secure.The use of
DOMPurify.sanitizebefore settingdangerouslySetInnerHTMLis the correct approach to prevent XSS attacks while allowing formatted translation content. The static analysis warning can be safely ignored here.apps/meteor/client/views/account/security/EndToEnd.tsx (1)
1-16: Excellent refactoring.The extraction of E2EE password management into
ChangePassphraseandResetPassphrasecomponents makesEndToEnda clean, focused container. This improves maintainability, testability, and follows the single responsibility principle.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (2)
184-184: Fix aria-required to be static.The confirmation field is always required per the validation rules (line 174), so
aria-requiredshould be'true'instead of toggling based on the passphrase value.Apply this diff:
- aria-required={passphrase ? 'true' : 'false'} + aria-required='true'
155-158: Add error handling for the async operation.The
e2e.decodePrivateKeyFlow()call can throw errors on invalid passwords or decryption failures. Wrap it in a try-catch to handle errors gracefully and provide user feedback.Apply this diff:
<Box is='a' href='#' - onClick={async (e) => { - e.preventDefault(); - await e2e.decodePrivateKeyFlow(); - }} + onClick={async (e) => { + e.preventDefault(); + try { + await e2e.decodePrivateKeyFlow(); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }} >
🧹 Nitpick comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
100-208: Consider wrapping in a semantic<form>element.Using a
<form>element instead of fragments would improve semantics, enable Enter key submission, and better integrate with react-hook-form'shandleSubmit. MovehandleSubmit(handleSave)to the form'sonSubmithandler.Example structure:
<form onSubmit={handleSubmit(handleSave)}> {/* existing Box and Field content */} <Button type='submit' primary disabled={!(keysExist && valid && isValid)} mbs={12} data-qa-type='e2e-encryption-save-password-button' > {t('Save_changes')} </Button> </form>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
📚 Learning: 2025-09-18T17:32:33.969Z
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
🧬 Code graph analysis (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (5)
apps/meteor/client/views/room/hooks/useE2EEState.ts (1)
useE2EEState(9-9)packages/password-policies/src/PasswordPolicy.ts (2)
validate(179-247)error(102-115)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(15-28)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(851-851)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
PasswordVerifierList(14-39)
🪛 ast-grep (0.39.7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[warning] 105-105: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[error] 106-106: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (3)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (3)
13-21: LGTM! Password policy correctly implements requirements.The policy definition correctly matches the PR objectives from ESH-43 for user-provided passwords: 30-character minimum with lowercase, uppercase, digit, and special character requirements.
106-106: Properly sanitized - static analysis false positive.The use of
DOMPurify.sanitize()correctly neutralizes XSS risks before rendering HTML. The static analysis warnings can be safely ignored.
129-135: LGTM! Conditional aria-describedby is correctly implemented.The aria-describedby construction properly matches the conditionally rendered elements: the verifier/hint IDs correspond to what
keysExistrenders, and the error ID is only included when the error exists. The.filter(Boolean)pattern correctly removes undefined values.
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (3)
155-158: Add error handling for the async operation.The
e2e.decodePrivateKeyFlow()call can throw errors (invalid password, decryption failure), but there's no try-catch block. This can result in unhandled promise rejections.Apply this diff to handle errors gracefully:
<Box is='a' href='#' - onClick={async (e) => { - e.preventDefault(); - await e2e.decodePrivateKeyFlow(); - }} + onClick={async (e) => { + e.preventDefault(); + try { + await e2e.decodePrivateKeyFlow(); + } catch (error) { + dispatchToastMessage({ type: 'error', message: error }); + } + }} >
184-184: Use staticaria-requiredvalue.The confirmation field is only rendered when
validistrue(line 166), which requires a non-empty passphrase that meets the policy. Therefore,passphraseis always truthy when this field exists, makingaria-requiredalways evaluate to'true'. The dynamic expression is confusing.Apply this diff:
- aria-required={passphrase ? 'true' : 'false'} + aria-required='true'
74-85: Addpassphraseto the useEffect dependency array to catch password mismatches.The confirmation field validation rule (line 175) compares against
passphrasevia closure:(value) => (passphrase !== value ? t('Passwords_do_not_match') : true). However, whenpassphrasechanges afterconfirmationPassphraseis already filled and valid, the effect does not run (sincevalidremainstrueandconfirmationPassphraseis unchanged). This leaves the form reportingisValideven though the passwords no longer match.The form's
mode: 'all'setting validates individual fields on interaction, but does not automatically re-validate one field when a different field's value changes. You must explicitly calltrigger()to force revalidation.Add
passphraseto the dependency array:-}, [valid, confirmationPassphrase, resetField, trigger]); +}, [passphrase, valid, confirmationPassphrase, resetField, trigger]);This ensures
trigger('confirmationPassphrase')is called wheneverpassphrasechanges andconfirmationPassphraseis populated, catching mismatches immediately.
🧹 Nitpick comments (1)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (1)
100-210: Consider wrapping fields in a<form>element for better semantics and accessibility.The current implementation uses a Fragment with button
onClickhandling. Using a<form>element with anonSubmithandler would provide:
- Semantic HTML structure for assistive technologies
- Native keyboard support (Enter key submission)
- Standard form behavior
- return ( - <> + return ( + <> <Box is='p' fontScale='p1' id={e2ePasswordExplanationId} dangerouslySetInnerHTML={{ __html: DOMPurify.sanitize(t('E2E_Encryption_Password_Explanation')) }} /> - <Box mbs={36} w='full'> + <Box is='form' mbs={36} w='full' onSubmit={handleSubmit(handleSave)}> <Box is='h3' fontScale='h4' mbe={12}> {t('Change_E2EE_password')} </Box> {/* ... fields ... */} <Button + type='submit' primary disabled={!(keysExist && valid && isValid)} - onClick={handleSubmit(handleSave)} mbs={12} data-qa-type='e2e-encryption-save-password-button' > {t('Save_changes')} </Button> </Box> + </> - ); + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx(1 hunks)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, the team has decided to use Latin-1 encoding (via Binary.toArrayBuffer and Binary.toString) for password encoding and decrypt output instead of UTF-8 encoding. This is a deliberate choice for E2EE password/key material handling.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
📚 Learning: 2025-10-07T15:08:37.419Z
Learnt from: cardoso
Repo: RocketChat/Rocket.Chat PR: 36942
File: apps/meteor/client/lib/e2ee/pbkdf2.ts:13-45
Timestamp: 2025-10-07T15:08:37.419Z
Learning: In apps/meteor/client/lib/e2ee/pbkdf2.ts, PBKDF2 iteration count validation is not enforced because the iterations parameter is outside the user's control and is system-managed.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
📚 Learning: 2025-09-18T17:32:33.969Z
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
Applied to files:
apps/meteor/client/views/account/security/ChangePassphrase.tsx
📚 Learning: 2025-09-23T19:22:59.217Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 36987
File: apps/meteor/tests/e2e/page-objects/fragments/room-toolbar.ts:10-20
Timestamp: 2025-09-23T19:22:59.217Z
Learning: In Playwright e2e tests, prefer stable selectors like data-qa-id attributes over localized text in getByRole() or getByText() calls to prevent test failures when UI language changes. Test translations separately by validating actual text content after ensuring UI interactions work with stable selectors.
Applied to files:
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx
🧬 Code graph analysis (2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (5)
apps/meteor/client/views/room/hooks/useE2EEState.ts (1)
useE2EEState(9-9)packages/password-policies/src/PasswordPolicy.ts (2)
validate(179-247)error(102-115)packages/ui-contexts/src/hooks/usePasswordPolicy.ts (1)
usePasswordPolicy(15-28)apps/meteor/client/lib/e2ee/rocketchat.e2e.ts (1)
e2e(851-851)packages/ui-client/src/components/PasswordVerifier/PasswordVerifierList.tsx (1)
PasswordVerifierList(14-39)
packages/ui-client/src/components/PasswordVerifier/PasswordVerifierItem.tsx (1)
packages/password-policies/src/PasswordPolicy.ts (1)
PasswordPolicyValidation(35-39)
🪛 ast-grep (0.39.7)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[warning] 105-105: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx
[error] 106-106: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (2)
apps/meteor/client/views/account/security/ChangePassphrase.tsx (2)
13-21: LGTM! Password policy aligns with requirements.The passphrase policy correctly enforces the 30-character minimum with mandatory character classes (uppercase, lowercase, number, special character) as specified in the linked issue ESH-43.
106-106: LGTM! DOMPurify sanitization is correctly applied.Static analysis flags the use of
dangerouslySetInnerHTML, but the code properly mitigates XSS risk by sanitizing the content withDOMPurify.sanitize()before rendering.
Adds complexity requirements to end-to-end encryption passphrase
cardoso
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Issue(s)
https://rocketchat.atlassian.net/browse/ESH-43
Steps to test or reproduce
Further comments
Summary by CodeRabbit
New Features
UI
Refactor
Tests