🐛 Fixed members inheriting stale suppression state on resignup

ref ONC-1640

When a member whose email was previously suppressed was deleted and a new
member signed up with the same address, the new member was created with
email_disabled=false despite the suppression record still existing. The
send filter then included them in every newsletter, Mailgun rejected each
send with a 607, and the handler silently swallowed the ER_DUP_ENTRY from
trying to re-insert the existing suppression record — so email_disabled
was never corrected.

- member-bread-service.add() now checks the suppression list and sets
  email_disabled, mirroring edit()
- mailgun-email-suppression-list handleEvent now dispatches
  EmailSuppressedEvent even on ER_DUP_ENTRY, self-healing drifted
  members on their next bounce/complaint

Author: jonatansberg

ghost/core/core/server/services/email-suppression-list/mailgun-email-suppression-list.js

@@ -117,31 +117,35 @@ class MailgunEmailSuppressionList extends AbstractEmailSuppressionList {
     async init() {
         this.Suppression = models.Suppression;
         const handleEvent = reason => async (event) => {
-            try {
-                if (reason === 'bounce') {
-                    if (!Number.isInteger(event.error?.code)) {
-                        return;
-                    }
-                    if (event.error.code !== 607 && event.error.code !== 605) {
-                        return;
-                    }
+            if (reason === 'bounce') {
+                if (!Number.isInteger(event.error?.code)) {
+                    return;
+                }
+                if (event.error.code !== 607 && event.error.code !== 605) {
+                    return;
                 }
+            }
+            try {
                 await this.Suppression.add({
                     email: event.email,
                     email_id: event.emailId,
                     reason: reason,
                     created_at: event.timestamp
                 });
-                DomainEvents.dispatch(EmailSuppressedEvent.create({
-                    emailAddress: event.email,
-                    emailId: event.emailId,
-                    reason: reason
-                }, event.timestamp));
             } catch (err) {
                 if (err.code !== 'ER_DUP_ENTRY') {
                     logging.error(err);
+                    return;
                 }
+                // Suppression already exists — still dispatch so any drifted
+                // member state (e.g. email_disabled=false) gets corrected.
+                logging.info(`Re-dispatching EmailSuppressedEvent for existing suppression (${reason}): ${event.email}`);
             }
+            DomainEvents.dispatch(EmailSuppressedEvent.create({
+                emailAddress: event.email,
+                emailId: event.emailId,
+                reason: reason
+            }, event.timestamp));
         };
         DomainEvents.subscribe(EmailBouncedEvent, handleEvent('bounce'));
         DomainEvents.subscribe(SpamComplaintEvent, handleEvent('spam'));

ghost/core/core/server/services/members/members-api/services/member-bread-service.js

@@ -348,6 +348,11 @@ module.exports = class MemberBREADService {
         let model;
 
         try {
+            if (data.email) {
+                const isSuppressed = (await this.emailSuppressionList.getSuppressionData(data.email))?.suppressed;
+                data.email_disabled = !!isSuppressed;
+            }
+
             const attribution = await this.memberAttributionService.getAttributionFromContext(options?.context);
             if (attribution) {
                 data.attribution = attribution;

ghost/core/test/integration/services/mailgun-email-suppression-list.test.js

@@ -5,6 +5,7 @@ const DomainEvents = require('@tryghost/domain-events');
 
 const MailgunClient = require('../../../core/server/services/lib/mailgun-client');
 const emailAnalytics = require('../../../core/server/services/email-analytics');
+const models = require('../../../core/server/models');
 
 describe('MailgunEmailSuppressionList', function () {
     let agent;
@@ -171,6 +172,46 @@ describe('MailgunEmailSuppressionList', function () {
         assert.equal(memberAfter.email_suppression.suppressed, true, 'The member should have a suppressed email');
         assert.equal(memberAfter.email_suppression.info.reason, 'spam');
     });
+
+    it('Sets email_disabled on the member when a suppression record already exists (ONC-1640)', async function () {
+        // Regression test: if a Suppression record already exists for the address
+        // (e.g. from a deleted previous member), a bounce event for a new member
+        // with the same address used to silently ER_DUP_ENTRY and never dispatch
+        // EmailSuppressedEvent, leaving the member with email_disabled=false forever.
+        const emailBatch = fixtureManager.get('email_batches', 0);
+        const emailRecipient = fixtureManager.get('email_recipients', 5);
+        assert(emailRecipient.batch_id === emailBatch.id);
+        const memberId = emailRecipient.member_id;
+        const providerId = emailBatch.provider_id;
+        const recipient = emailRecipient.member_email;
+        const timestamp = new Date(2000, 0, 1);
+
+        await models.Suppression.add({
+            email: recipient,
+            email_id: emailBatch.email_id,
+            reason: 'bounce',
+            created_at: new Date(1999, 0, 1)
+        });
+
+        const memberBefore = await models.Member.findOne({id: memberId}, {require: true});
+        assert.equal(memberBefore.get('email_disabled'), false, 'Test setup: the member should start with email_disabled=false');
+
+        events = [createPermanentFailedEvent({
+            errorCode: 605,
+            providerId,
+            timestamp,
+            recipient
+        })];
+
+        await emailAnalytics.fetchLatestOpenedEvents();
+        await DomainEvents.allSettled();
+
+        const memberAfter = await models.Member.findOne({id: memberId}, {require: true});
+        assert.equal(memberAfter.get('email_disabled'), true, 'Member should be disabled even when the Suppression record already existed');
+
+        // Clean up so subsequent tests using this email aren't affected
+        await models.Suppression.destroy({destroyBy: {email: recipient}});
+    });
 });
 
 function createPermanentFailedEvent({errorCode, providerId, timestamp, recipient}) {

ghost/core/test/unit/server/services/members/members-api/services/members-bread-service.test.js

@@ -45,6 +45,7 @@ describe('MemberBreadService', function () {
 
             const linkStripeCustomerStub = sinon.stub().resolves();
             const createStub = sinon.stub().resolves(mockMemberModel);
+            const getSuppressionDataStub = sinon.stub().resolves({suppressed: false, info: null});
 
             const memberRepository = {
                 create: createStub,
@@ -60,7 +61,7 @@ describe('MemberBreadService', function () {
                 labsService: {isSet: sinon.stub().returns(false)},
                 newslettersService: {browse: sinon.stub().resolves([])},
                 settingsCache: {get: sinon.stub()},
-                emailSuppressionList: {getSuppressionData: sinon.stub().resolves({suppressed: false, info: null})},
+                emailSuppressionList: {getSuppressionData: getSuppressionDataStub},
                 settingsHelpers: {createUnsubscribeUrl: sinon.stub().returns('http://example.com/unsubscribe')}
             });
 
@@ -72,7 +73,7 @@ describe('MemberBreadService', function () {
                 status: 'free'
             });
 
-            return {service, memberRepository, linkStripeCustomerStub, createStub};
+            return {service, memberRepository, linkStripeCustomerStub, createStub, getSuppressionDataStub};
         }
 
         it('passes context to linkStripeCustomer when stripe_customer_id is provided', async function () {
@@ -149,6 +150,107 @@ describe('MemberBreadService', function () {
             assert.ok(linkStripeCustomerOptions.context, 'context should be passed to linkStripeCustomer');
             assert.equal(linkStripeCustomerOptions.context.import, true, 'context.import should be true');
         });
+
+        it('sets email_disabled to true when the email is on the suppression list', async function () {
+            // Prevents ONC-1640: a previous member with this email bounced/complained,
+            // was deleted, and the suppression record remains. A new signup with that
+            // same address must inherit the disabled state instead of starting clean.
+            const {service, createStub, getSuppressionDataStub} = createService();
+            getSuppressionDataStub.resolves({suppressed: true, info: {reason: 'spam'}});
+
+            await service.add({
+                email: 'suppressed@example.com',
+                name: 'New Signup'
+            }, {});
+
+            assert.equal(createStub.calledOnce, true);
+            const createdData = createStub.firstCall.args[0];
+            assert.equal(createdData.email_disabled, true);
+        });
+
+        it('sets email_disabled to false when the email is not on the suppression list', async function () {
+            const {service, createStub} = createService();
+
+            await service.add({
+                email: 'clean@example.com',
+                name: 'Clean Signup'
+            }, {});
+
+            assert.equal(createStub.calledOnce, true);
+            const createdData = createStub.firstCall.args[0];
+            assert.equal(createdData.email_disabled, false);
+        });
+    });
+
+    describe('edit', function () {
+        function createMockMemberModel() {
+            return {
+                id: 'member_123',
+                get: sinon.stub().returns(false),
+                related: sinon.stub().returns({find: () => null, toJSON: () => [], models: []}),
+                toJSON: sinon.stub().returns({
+                    id: 'member_123',
+                    email: 'test@example.com'
+                })
+            };
+        }
+
+        function createService() {
+            const mockMemberModel = createMockMemberModel();
+            const updateStub = sinon.stub().resolves(mockMemberModel);
+            const getSuppressionDataStub = sinon.stub().resolves({suppressed: false, info: null});
+
+            const service = new MemberBreadService({
+                memberRepository: {
+                    update: updateStub
+                },
+                stripeService: {configured: false},
+                memberAttributionService: {getAttributionFromContext: sinon.stub().resolves(null)},
+                emailService: {},
+                labsService: {isSet: sinon.stub().returns(false)},
+                newslettersService: {browse: sinon.stub().resolves([])},
+                settingsCache: {get: sinon.stub()},
+                emailSuppressionList: {getSuppressionData: getSuppressionDataStub},
+                settingsHelpers: {createUnsubscribeUrl: sinon.stub().returns('http://example.com/unsubscribe')}
+            });
+
+            sinon.stub(service, 'read').resolves({id: 'member_123'});
+
+            return {service, updateStub, getSuppressionDataStub};
+        }
+
+        it('sets email_disabled to true when the new email is on the suppression list', async function () {
+            const {service, updateStub, getSuppressionDataStub} = createService();
+            getSuppressionDataStub.resolves({suppressed: true, info: {reason: 'spam'}});
+
+            await service.edit({
+                email: 'suppressed@example.com'
+            }, {id: 'member_123'});
+
+            const updatedData = updateStub.firstCall.args[0];
+            assert.equal(updatedData.email_disabled, true);
+        });
+
+        it('sets email_disabled to false when the new email is not on the suppression list', async function () {
+            const {service, updateStub} = createService();
+
+            await service.edit({
+                email: 'clean@example.com'
+            }, {id: 'member_123'});
+
+            const updatedData = updateStub.firstCall.args[0];
+            assert.equal(updatedData.email_disabled, false);
+        });
+
+        it('does not check the suppression list when email is not being changed', async function () {
+            const {service, getSuppressionDataStub} = createService();
+
+            await service.edit({
+                name: 'New Name'
+            }, {id: 'member_123'});
+
+            assert.equal(getSuppressionDataStub.called, false);
+        });
     });
 
     describe('read', function () {