Skip to content

Fix:- update the regex for password validation #3070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Fix:- update the regex for password validation #3070

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion server/src/validation/joi.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const roleValidatior = (role) => (value, helpers) => {
//****************************************

const passwordPattern =
/^(?=.*[a-z])(?=.*[A-Z])(?=.*[0-9])(?=.*[!?@#$%^&*()\-_=+[\]{};:'",.<>~`|\\/])[A-Za-z0-9!?@#$%^&*()\-_=+[\]{};:'",.<>~`|\\/]+$/;
/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[!@#$%^&*_\-+=\\[\]{};:'~,./|])(?!.*[<>`"])[A-Za-z\d!@#$%^&*_\-+=\\[\]{};:'~,./|]+$/;
Comment on lines 20 to +21
Copy link

@coderabbitai coderabbitai bot Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Critical: This fix contradicts the stated objective from issue #3010.

The linked issue #3010 requests: "Ensure successful user creation when the first character of the password is '<' (or other valid symbols)". However, this implementation explicitly disallows passwords containing <, >, backtick, or " anywhere in the string via the negative lookahead (?!.*[<>"])`.

This changes the requirement rather than fixing the bug. The original error "password cannot be empty" was misleading, but the solution should either:

  1. Allow these characters and fix the root cause of why < was misinterpreted, OR
  2. Update issue 'Password cannot be empty' user signup bug on fresh install #3010 to reflect that these characters are intentionally disallowed for security reasons

Additional concerns:

  1. Redundant negative lookahead: The characters disallowed by (?!.*[<>"])are already absent from the allowed character class[A-Za-z\d!@#$%^&*_-+=\[]{};:'~,./|]`, so they would be rejected anyway. The negative lookahead adds no functional value.

  2. Potential security issue: Disallowing HTML-sensitive characters (<, >, ", backtick) in passwords suggests they might be improperly handled elsewhere (e.g., displayed without escaping or used in HTML contexts without sanitization). Passwords should always be hashed and never rendered in HTML, making these restrictions unnecessary.

  3. Poor user experience: The generic Joi pattern validation error won't clearly indicate which characters are problematic. Users won't understand why their password is rejected.

Please clarify the actual requirement and verify password handling practices:

🏁 Script executed:

#!/bin/bash
# Search for potential password mishandling - displaying, logging, or using passwords in HTML contexts

echo "=== Searching for potential password display or logging ==="
rg -n -C3 "password" --type=js -g '!node_modules' -g '!*.test.js' -g '!*.spec.js' | rg -i "(console\.|log|display|render|html|innerHTML)"

echo -e "\n=== Checking if passwords are being compared in plaintext ==="
ast-grep --pattern $'if ($PASSWORD === $_) { $$$ }'

echo -e "\n=== Checking password hashing implementation ==="
rg -n -C3 "bcrypt|argon2|scrypt|hash.*password" --type=js -g '!node_modules'

Length of output: 103973

Critical: This implementation contradicts issue #3010's stated objective.

Issue #3010 requests allowing passwords that start with < (or other valid symbols). However, this implementation explicitly disallows passwords containing <, >, backtick, or " via the negative lookahead (?!.*[<>"])` at line 21.

This reverses the requirement rather than fixing the underlying issue.

Redundant negative lookahead:

The characters blocked by (?!.*[<>"])are already absent from the allowed character class[A-Za-z\d!@#$%^&*_-+=\[]{};:'~,./|]`, making the negative lookahead functionally unnecessary.

Required action:

Clarify the intent: either remove the negative lookahead and allow these characters (fixing the actual bug), or update issue #3010 to reflect that certain characters are intentionally restricted and document the business reason.

🤖 Prompt for AI Agents 
In server/src/validation/joi.js around lines 20-21 the regex contains a
redundant negative lookahead (?!.*[<>`"]) that both contradicts issue #3010 and
is unnecessary because those characters are already excluded by the
allowed-character class; to fix, remove the negative lookahead and update the
allowed-character class to permit the intended characters (e.g., include <, >,
backtick, and double-quote if they should be allowed) so passwords starting with
those symbols pass validation, then add/adjust unit tests and a short comment
documenting the intended character policy; if the intent was to keep them banned
instead, update issue #3010 with the business justification and document the
restriction in code comments.

Copy link

@llamapreview llamapreview bot Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 | Confidence: High

  • Security: The password validation regex has been updated to explicitly disallow characters <, >, `, and " using a negative lookahead. This addresses the security vulnerability where these characters were being sanitized by the server's input sanitization middleware, causing passwords starting with < to become empty. However, this change represents a breaking modification to the password policy by removing previously allowed special characters.
  • Architecture: This change addresses a symptom (password validation) rather than the root cause (overly aggressive input sanitization). A more architectural approach would be to modify the sanitization middleware to handle password fields differently, such as excluding them from sanitization or using context-aware strategies.
  • Maintainability: The regex complexity has increased significantly with the addition of negative lookahead and character class changes. Complex regex patterns are difficult to maintain, test, and understand, making it harder to modify individual requirements.
  • Testing: Given the significant changes to allowed special characters and the addition of character blacklisting, comprehensive test coverage is crucial. Ensure tests cover edge cases, previously allowed characters that are now disallowed, and all positive validation requirements.
Suggested change
/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[!@#$%^&*_\-+=\\[\]{};:'~,./|])(?!.*[<>`"])[A-Za-z\d!@#$%^&*_\-+=\\[\]{};:'~,./|]+$/;
const hasLowerCase = /[a-z]/;
const hasUpperCase = /[A-Z]/;
const hasDigit = /\d/;
const hasSpecialChar = /[!@#$%^&*_\-+=\[\]{};:'~,./|]/;
const hasInvalidChars = /[<>`"]/;
function validatePassword(password) {
return hasLowerCase.test(password) &&
hasUpperCase.test(password) &&
hasDigit.test(password) &&
hasSpecialChar.test(password) &&
!hasInvalidChars.test(password);
}

Evidence: path:server/src/v1/middleware/sanitization.js


const loginValidation = joi.object({
email: joi.string().email().required().lowercase(),
Expand Down