RocketChat / RocketChat/Rocket.Chat

[bug]: Add comprehensive tests for PasswordPolicy.sendValidationMessage() and fix null input crash

Open
#42,149 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: bug type: feature
Dominant language
TypeScript
Stars
46.1k
Forks
13.9k
Avg merge
3d 3h
Merged PRs (30d)
130

Description

Description

The sendValidationMessage() method in @rocket.chat/password-policies has zero test coverage, despite being actively used in production by usePasswordPolicy to drive the real-time password strength validation UI.

Additionally, the method has an edge case bug: calling sendValidationMessage(null) or sendValidationMessage(undefined) throws an unhandled TypeError because it accesses .length without a type guard, unlike validate() which has a guard on line 185.

Current behavior

  1. sendValidationMessage() has zero tests in PasswordPolicy.spec.ts.
  2. sendValidationMessage(null as any) throws TypeError: Cannot read properties of null (reading 'length').
  3. PasswordPolicyError is not exported from the package's public API (src/index.ts), preventing consumers from performing instanceof checks.
  4. throwError: true mode (the server default) is completely untested.

Expected behavior

  1. sendValidationMessage() should be tested for every policy rule.
  2. sendValidationMessage() should handle null/undefined gracefully, consistent with validate().
  3. PasswordPolicyError should be exported.
  4. Error throwing mode should be tested with proper assertions on error shape.

Relevant files

  • packages/password-policies/src/PasswordPolicy.ts (lines 117-177: sendValidationMessage, line 185: the validate type guard for comparison)
  • packages/password-policies/src/PasswordPolicy.spec.ts (existing tests - no mention of sendValidationMessage)
  • packages/password-policies/src/PasswordPolicyError.ts (not exported from index.ts)
  • packages/password-policies/src/index.ts (missing PasswordPolicyError export)

Proposed solution

  1. Add a describe('sendValidationMessage') test suite covering:
    • Each policy rule returning correct isValid and name properties
    • null, undefined, and empty string inputs
    • Disabled policy returning empty array
  2. Add a type guard in sendValidationMessage() for non-string / empty input
  3. Add export { PasswordPolicyError } from './PasswordPolicyError'; to src/index.ts
  4. Add tests for throwError: true mode verifying PasswordPolicyError properties
  5. Add this.name = 'PasswordPolicyError' in the error constructor

Acceptance criteria

  • sendValidationMessage() has tests covering all 7 policy rules
  • sendValidationMessage(null) and sendValidationMessage(undefined) no longer throw TypeError
  • PasswordPolicyError is exported from src/index.ts
  • throwError: true mode is tested with assertions on .error, .message, and .details
  • All existing tests continue to pass
  • yarn workspace @rocket.chat/password-policies testunit passes

Hi! I noticed this test coverage gap and the small edge cases while exploring the repository. If this issue looks reasonable, I'd be happy to work on it as my first contribution. Could you please assign it to me?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with packages/password-policies/src/PasswordPolicy.spec.ts and compare the existing validate() guard in packages/password-policies/src/PasswordPolicy.ts with sendValidationMessage(). Review PasswordPolicyError.ts and src/index.ts, then run yarn workspace @rocket.chat/password-policies testunit. Done means all seven rules, null/undefined inputs, disabled policy behavior, throwError mode, and the public error export are covered and the tests pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
security, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.