Skip to content

Tighten the notification email validator against header injection and add tests #570

Description

@mikewheeleer

Tighten the notification email validator against header injection and add tests

Description

isValidEmail in src/services/notification.service.ts rejects CR/LF but otherwise uses the loose ^[^\s@]+@[^\s@]+\.[^\s@]+$ pattern, so addresses containing quotes, backslashes, and other characters that SMTP can misinterpret pass validation, and the helper has no dedicated tests. Since validated addresses flow into the (soon real) email transport, a permissive validator is both a correctness and an injection concern. This issue tightens the rule and locks the behaviour down with tests.

Requirements and context

  • Repository scope: Talenttrust/Talenttrust-Backend only.
  • Strengthen isValidEmail to reject control characters, comma/semicolon-separated multi-recipients, and quoting/backslash forms that enable header or recipient injection, while still accepting normal RFC-shaped addresses.
  • Keep the CR/LF rejection and the boolean contract; do not change the method signature.
  • Ensure the validator is applied before any dispatch in the notification path.
  • Keep behaviour deterministic and documented so the email transport work can rely on it.

Suggested execution

  • Fork the repo and create a branch
  • git checkout -b test/notification-email-validator-hardening
  • Implement changes
    • Write code in: src/services/notification.service.ts.
    • Write comprehensive tests in: create src/services/notification.email-validation.test.ts — assert rejection of CR/LF, multi-recipient, quoted/backslash forms, and acceptance of valid addresses.
    • Add documentation: note the validation rules in docs/email-notifications.md.
    • Add TSDoc to isValidEmail.
    • Validate security: no header/recipient-injection form passes the validator.
  • Test and commit

Test and commit

  • Run npm run lint and npm run test:ci.
  • Cover edge cases: embedded newline, a@b,c@d, "x"@y.com, valid user@example.com, missing TLD.
  • Include the full npm test output and a security notes section in the PR.

Example commit message

test(notifications): harden and cover the email validator

Guidelines

  • Minimum 95 percent test coverage for impacted modules.
  • Clear, reviewer-focused documentation.
  • Timeframe: 96 hours.

Community & contribution rewards

  • 💬 Join the TalentTrust community on Discord for questions, reviews, and faster merges: https://discord.gg/WqnGpcPx
  • ⭐ This is a GrantFox OSS / Official Campaign task and may be rewarded. When your PR is merged you'll be prompted to rate the project — if this issue and the maintainers helped you ship, we'd be grateful for a 5-star rating. Clear questions in Discord and tidy, well-tested PRs are the fastest path to a merge and a reward.

Metadata

Metadata

Assignees

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions