feat: configurable brute-force lockout, email sanitization, localized risk flags, sanitized errors - #1226
Merged
mftee merged 2 commits intoAug 26, 2026
Conversation
… risk flags, sanitized error responses
Contributor
|
@northersubair is attempting to deploy a commit to the Mftee's projects Team on Vercel. A member of the Team first needs to authorize it. |
mftee
approved these changes
Aug 26, 2026
mftee
left a comment
Contributor
There was a problem hiding this comment.
Reviewed. Good work:
- BruteForceGuard lockout duration is now configurable via BRUTE_FORCE_LOCK_MINUTES (defaulting to 15) instead of hardcoded.
- Real trim/lowercase normalization on RegisterAuthDto email and trim on fullName via class-transformer, with tests validating both the transform and the existing validators still fire correctly.
- Risk flag i18n: real, non-stub French/Spanish/English translations for every RiskFlag value, with a sensible fallback to English for missing/unknown locales.
Two things worth flagging:
GET /:id/riskresponse shape changed:flagswasRiskFlag[](bare enum strings) and is nowRiskFlagResult[]({flag, description}objects). This is a breaking wire-contract change for any existing frontend consumer expecting bare strings — worth checking nothing currently rendersflagsassuming string values.- Minor: the new sensitive-message sanitization list includes
/e\.?r\.?r\.?o\.?r\.?/i, which functionally just matches the substring 'error' anywhere (case-insensitive) — so any production error message that happens to contain the word 'error' (even a benign one like 'Invalid error code provided') gets replaced with the generic message. Not broken, just possibly more aggressive than intended; worth confirming that's the desired behavior.
Approving.
Resolve conflicts: - brute-force.guard.ts: merged this PR's configurable BRUTE_FORCE_LOCK_MINUTES into CodeGirlsInc#1224's already-merged Redis-backed guard (kept Redis implementation, made its lock TTL configurable instead of hardcoded). - brute-force.guard.spec.ts (add/add): this PR's version tested the old synchronous in-memory guard, now obsolete after CodeGirlsInc#1224's Redis rewrite. Kept CodeGirlsInc#1224's Redis-mocked version. - risk-assessment.service.spec.ts (add/add): critical fix here - CodeGirlsInc#1225's already-merged risk-scoring test suite asserted expect(result.flags).toContain(RiskFlag.X) directly, but this PR changes RiskResult.flags from RiskFlag[] to RiskFlagResult[] ({flag, description} objects). Left as-is, all 17 of those assertions would have started failing the moment this PR merged. Rewrote them to expect(result.flags.map(f => f.flag)).toContain(...) and combined with this PR's localization tests into one file.
|
@northersubair Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1145
Closes #1144
Closes #1143
Closes #1142