fix: Use modern email validator to support new gTLDs (.rocks, .radio, .app)#27136
fix: Use modern email validator to support new gTLDs (.rocks, .radio, .app)#27136cyphercodes wants to merge 3 commits intoTryGhost:mainfrom
Conversation
… .app)
Changed all validator.isEmail() calls to use {legacy: false} option,
which enables support for modern generic Top-Level Domains like
.rocks, .radio, .app that were approved by ICANN after 2012.
The legacy validator (validator@7.2.0 from 2017) predates many gTLDs
and rejects valid email addresses using these domains.
Files changed:
- email-address-service-wrapper.js
- automated_emails.js
- invitations.js
- password_reset.js
- settings.js
Fixes TryGhost#26197
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughCalls to 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js`:
- Line 76: Update the two member-facing validators to use the modern email rules
by adding the options object {legacy: false} to the validator.isEmail calls:
locate the email validation in magic-link.js (the function that validates the
magic-link/email input) and the email check in router-controller.js (the
controller handling member signup/signin) and change their
validator.isEmail(email) calls to validator.isEmail(email, {legacy: false}),
keeping any surrounding typeof/email presence checks intact so behavior matches
the other validators (e.g., automated_emails.js).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4b81fdd4-def0-4bc1-b71a-eba7fd44667b
📒 Files selected for processing (5)
ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.jsghost/core/core/server/api/endpoints/utils/validators/input/invitations.jsghost/core/core/server/api/endpoints/utils/validators/input/password_reset.jsghost/core/core/server/api/endpoints/utils/validators/input/settings.jsghost/core/core/server/services/email-address/email-address-service-wrapper.js
| const lexical = frame.data.lexical; | ||
|
|
||
| if (typeof email !== 'string' || !validator.isEmail(email)) { | ||
| if (typeof email !== 'string' || !validator.isEmail(email, {legacy: false})) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify member-facing email validation callsites still using default isEmail(...)
rg -nP --type=js -C2 '\bisEmail\s*\(\s*[^,()]+\s*\)' \
ghost/core/core/server/services/members/members-api/controllers/router-controller.js \
ghost/core/core/server/services/lib/magic-link/magic-link.js
# Compare with explicit modern validation usage added in this PR area
rg -nP --type=js -C2 '\bisEmail\s*\(\s*[^,()]+\s*,\s*\{\s*legacy\s*:\s*false\s*\}\s*\)' \
ghost/core/core/server/api/endpoints/utils/validators/input \
ghost/core/core/server/services/email-address/email-address-service-wrapper.jsRepository: TryGhost/Ghost
Length of output: 4390
Member authentication flows still use legacy email validation.
Two member-facing entry points (magic-link.js line 75 and router-controller.js line 706) still call isEmail(...) without { legacy: false }, while the current file and other API validators use the modern validation. This allows modern gTLDs to slip through validation in signup and signin flows, creating an inconsistent email validation scope.
These files need to adopt the same { legacy: false } parameter:
ghost/core/core/server/services/lib/magic-link/magic-link.jsghost/core/core/server/services/members/members-api/controllers/router-controller.js
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ghost/core/core/server/api/endpoints/utils/validators/input/automated_emails.js`
at line 76, Update the two member-facing validators to use the modern email
rules by adding the options object {legacy: false} to the validator.isEmail
calls: locate the email validation in magic-link.js (the function that validates
the magic-link/email input) and the email check in router-controller.js (the
controller handling member signup/signin) and change their
validator.isEmail(email) calls to validator.isEmail(email, {legacy: false}),
keeping any surrounding typeof/email presence checks intact so behavior matches
the other validators (e.g., automated_emails.js).
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
refs TryGhost#26197 Member sign-in and magic-link flows still used legacy email validation.
|
Pushed a follow-up commit addressing the missed member sign-in validation paths. Summary:
Verification:
Note: I also attempted to update/rebase the branch onto latest |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ghost/core/test/unit/server/services/members/members-api/controllers/router-controller.test.js (1)
1011-1022: Test placement and assertions could be tighter.This test lives in the
honeypotblock and setshoneypot = 'filled!', so it implicitly relies on email validation passing before the honeypot short-circuit (since validation at line 706 ofrouter-controller.jsruns before the honeypot check at line 720). It does verify the gTLD passes validation (otherwise aBadRequestErrorwould propagate), but the test name suggests this is purely a gTLD-acceptance test.Consider either:
- Adding a complementary test in a more general
email validationdescribe block (without honeypot) that assertssendEmailWithMagicLinkis called forjamie@example.rocks, which more directly proves modern gTLDs are accepted in the actual signup flow.- Or renaming to clarify it's the honeypot-with-modern-gTLD scenario.
Not a blocker — the current test does provide coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ghost/core/test/unit/server/services/members/members-api/controllers/router-controller.test.js` around lines 1011 - 1022, The test titled "Accepts email addresses with modern gTLDs" lives in the honeypot block and therefore short-circuits via req.body.honeypot='filled!', so add a complementary test in the email validation describe block (or rename the existing test) that calls createRouterController() and controller.sendMagicLink(req, res) with req.body.email='jamie@example.rocks' and req.body.honeypot='' (or omitted), then assert sendEmailWithMagicLinkStub was called (e.g. sendEmailWithMagicLinkStub.calledOnce) and the response is 201 JSON; this proves modern gTLDs pass normal validation rather than relying on the honeypot path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@ghost/core/test/unit/server/services/members/members-api/controllers/router-controller.test.js`:
- Around line 1011-1022: The test titled "Accepts email addresses with modern
gTLDs" lives in the honeypot block and therefore short-circuits via
req.body.honeypot='filled!', so add a complementary test in the email validation
describe block (or rename the existing test) that calls createRouterController()
and controller.sendMagicLink(req, res) with req.body.email='jamie@example.rocks'
and req.body.honeypot='' (or omitted), then assert sendEmailWithMagicLinkStub
was called (e.g. sendEmailWithMagicLinkStub.calledOnce) and the response is 201
JSON; this proves modern gTLDs pass normal validation rather than relying on the
honeypot path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: bff1cff9-acb5-4d86-a9f0-6a52ed97205b
📒 Files selected for processing (4)
ghost/core/core/server/services/lib/magic-link/magic-link.jsghost/core/core/server/services/members/members-api/controllers/router-controller.jsghost/core/test/unit/server/services/lib/magic-link/index.test.jsghost/core/test/unit/server/services/members/members-api/controllers/router-controller.test.js
✅ Files skipped from review due to trivial changes (1)
- ghost/core/core/server/services/lib/magic-link/magic-link.js
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 17ef92b. Configure here.
| sinon.assert.calledWith(res.writeHead, 201, {'Content-Type': 'application/json'}); | ||
| assert.equal(res.end.calledOnceWith('{}'), true); | ||
| assert.equal(sendEmailWithMagicLinkStub.notCalled, true); | ||
| }); |
There was a problem hiding this comment.
Test uses honeypot, never verifies email actually sends
Medium Severity
The test "Accepts email addresses with modern gTLDs" sets req.body.honeypot = 'filled!', which causes the controller to take the bot-detection shortcut path (return 201 without sending). The test then asserts sendEmailWithMagicLinkStub.notCalled, effectively verifying the email is not sent — the opposite of what "accepts" implies. While it does indirectly confirm validation passes, it gives false confidence that the full send path works with modern gTLDs. Removing the honeypot and asserting the stub is called would properly test acceptance.
Reviewed by Cursor Bugbot for commit 17ef92b. Configure here.
|
You have used all of your free Bugbot PR reviews. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
|
Updated the router-controller coverage for the review feedback:
Verification:
|
1 participant
Summary
Fixed email validation to support modern generic Top-Level Domains (gTLDs) like
.rocks,.radio,.app, etc.Problem
Ghost's email validation was rejecting legitimate email addresses using newer gTLDs approved by ICANN after 2012. The legacy validator (validator@7.2.0 from 2017) predates many of these TLDs.
Solution
Changed all
validator.isEmail()calls to use{legacy: false}option, which enables the modern custom validator that properly handles all TLDs.Files Changed
email-address-service-wrapper.jsautomated_emails.jsinvitations.jspassword_reset.jssettings.jsTesting
Fixes #26197
Note
Medium Risk
Changes email validation behavior across several auth/members-related endpoints (magic links, invitations, password reset, settings), which could affect who can trigger these flows. Risk is moderate because it broadens accepted input but doesn’t alter core authorization or token logic.
Overview
Switches multiple server-side email validation call sites to use
validator.isEmail(..., {legacy: false}), enabling support for modern gTLDs across automated email test sends, invitations, password reset token generation, member support address settings, managed email address validation, and magic-link sign-in/signup flows.Adds unit coverage to ensure magic-link sending and the members
sendMagicLinkcontroller accept addresses likeexample.rocks.Reviewed by Cursor Bugbot for commit 17ef92b. Bugbot is set up for automated code reviews on this repo. Configure here.