Skip to content

Commit 782df4e

Browse files
fix(mailer): return a generic client message on resend mail-transport failures
resendVerification (auth.controller.js) and the invitations resend endpoint now surface propagated mail-transport failures via errors.getMessage(err), which relayed the raw SMTP/Resend provider error string to the client on these two endpoints — a low-severity infra-detail leak flagged by the pre-push critical-review gate on #3966. Both catches now log the real error server-side with context (userId / invitationId) and respond with a stable generic message. invitations.resend still threads deliberate AppError statuses (409 conflict, 404 not found, 422 mailer-not-configured) through unchanged — only the untagged transport rejection (no `.status`) gets the generic treatment. resendVerification's catch is resend-email-scoped end-to-end, so the whole catch goes generic. Claude-Session: https://claude.ai/code/session_01WfNC8bt1TgL4AsiYgCEGup
1 parent 219897a commit 782df4e

4 files changed

Lines changed: 186 additions & 5 deletions

File tree

modules/auth/controllers/auth.controller.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -850,7 +850,17 @@ const resendVerification = async (req, res) => {
850850
if (!acceptedCount) return responses.error(res, 400, 'Bad Request', 'Failure sending email')();
851851
return responses.success(res, 'Verification email sent')({ status: true });
852852
} catch (err) {
853-
responses.error(res, 422, 'Unprocessable Entity', errors.getMessage(err))(err);
853+
// #3966 hardening: sendVerificationEmail (mailer.sendMail) now propagates a
854+
// transport failure instead of swallowing it — the raw SMTP/provider error
855+
// string must not leak to the client. Log the real error server-side with
856+
// context (this catch is resend-email-scoped: an unexpected failure here is
857+
// effectively always the mail send); respond with a stable generic message.
858+
logger.error('[auth.resendVerification] failed', {
859+
userId: req.user?.id,
860+
message: err?.message,
861+
stack: err?.stack,
862+
});
863+
responses.error(res, 422, 'Unprocessable Entity', 'Failed to send the email, please try again.')(err);
854864
}
855865
};
856866

modules/auth/tests/auth.silent.catch.unit.tests.js

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,3 +411,137 @@ describe('auth.password.controller silent-catch error logging:', () => {
411411
});
412412
});
413413
});
414+
415+
describe('auth.controller resendVerification mail-transport failure hardening (#3966):', () => {
416+
let mockError;
417+
418+
beforeEach(() => {
419+
jest.resetModules();
420+
421+
mockError = jest.fn();
422+
423+
jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
424+
default: { warn: jest.fn(), error: mockError, info: jest.fn() },
425+
}));
426+
});
427+
428+
test('responds with a generic message (never the raw provider error) and still logs the real error server-side', async () => {
429+
// sendVerificationEmail → mailer.sendMail is awaited directly (not
430+
// fire-and-forget) and now propagates a transport failure (#3966) instead
431+
// of swallowing it — the controller catch must not leak this raw string.
432+
const providerError = new Error('Resend API error: 401 Unauthorized — invalid API key sk_live_abc123');
433+
434+
jest.unstable_mockModule('../../../modules/users/services/users.service.js', () => ({
435+
default: {
436+
create: jest.fn(),
437+
getBrut: jest.fn().mockResolvedValue({ id: 'u1', email: 'x@y.com', emailVerified: false }),
438+
update: jest.fn().mockResolvedValue({}),
439+
remove: jest.fn(),
440+
count: jest.fn().mockResolvedValue(0),
441+
},
442+
}));
443+
444+
jest.unstable_mockModule('../../../modules/auth/services/auth.eligibility.js', () => ({
445+
default: {
446+
registerSignupEligibility: jest.fn(),
447+
assertSignupEligible: jest.fn().mockResolvedValue(undefined),
448+
_reset: jest.fn(),
449+
},
450+
}));
451+
452+
jest.unstable_mockModule('../../../modules/organizations/services/organizations.service.js', () => ({
453+
default: { handleSignupOrganization: jest.fn() },
454+
}));
455+
456+
jest.unstable_mockModule('../../../modules/organizations/services/organizations.crud.service.js', () => ({
457+
default: { autoSetCurrentOrganization: jest.fn() },
458+
}));
459+
460+
jest.unstable_mockModule('../../../modules/organizations/services/organizations.membership.service.js', () => ({
461+
default: { findByUserAndOrganization: jest.fn(), listPendingByUser: jest.fn().mockResolvedValue([]) },
462+
}));
463+
464+
jest.unstable_mockModule('../../../config/index.js', () => ({
465+
default: {
466+
sign: { up: true, in: true },
467+
jwt: { secret: 'test-secret', expiresIn: 3600 },
468+
cookie: { secure: false, sameSite: 'lax' },
469+
organizations: { enabled: false },
470+
app: { title: 'Test', contact: 'test@test.com' },
471+
},
472+
}));
473+
474+
jest.unstable_mockModule('../../../lib/middlewares/model.js', () => ({
475+
default: { getResultFromZod: jest.fn(), checkError: jest.fn() },
476+
}));
477+
478+
// Mailer configured, sendMail rejects with a raw provider error
479+
jest.unstable_mockModule('../../../lib/helpers/mailer/index.js', () => ({
480+
default: {
481+
isConfigured: jest.fn().mockReturnValue(true),
482+
sendMail: jest.fn().mockRejectedValue(providerError),
483+
},
484+
}));
485+
486+
const successInner = jest.fn();
487+
const errorInner = jest.fn();
488+
const success = jest.fn(() => successInner);
489+
const error = jest.fn(() => errorInner);
490+
jest.unstable_mockModule('../../../lib/helpers/responses.js', () => ({
491+
default: { success, error },
492+
}));
493+
494+
jest.unstable_mockModule('../../../lib/helpers/errors.js', () => ({
495+
default: { getMessage: jest.fn().mockReturnValue(providerError.message) },
496+
}));
497+
498+
jest.unstable_mockModule('../../../lib/helpers/AppError.js', () => ({
499+
default: class AppError extends Error {
500+
constructor(msg, opts) {
501+
super(msg);
502+
this.status = opts?.status;
503+
this.code = opts?.code;
504+
this.details = opts?.details;
505+
}
506+
},
507+
}));
508+
509+
jest.unstable_mockModule('../../../modules/users/models/users.schema.js', () => ({
510+
default: { User: {} },
511+
}));
512+
513+
jest.unstable_mockModule('../../../lib/middlewares/policy.js', () => ({
514+
default: { defineAbilityFor: jest.fn().mockResolvedValue({}) },
515+
}));
516+
517+
jest.unstable_mockModule('../../../lib/helpers/abilities.js', () => ({
518+
default: jest.fn().mockReturnValue([]),
519+
}));
520+
521+
jest.unstable_mockModule('../../../lib/helpers/getBaseUrl.js', () => ({
522+
default: jest.fn().mockReturnValue('http://localhost:3000'),
523+
}));
524+
525+
jest.unstable_mockModule('../../../lib/services/analytics.js', () => ({
526+
default: { identify: jest.fn(), groupIdentify: jest.fn(), capture: jest.fn() },
527+
}));
528+
529+
const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
530+
531+
const req = { user: { id: 'u1' } };
532+
const res = {};
533+
534+
await AuthController.resendVerification(req, res);
535+
536+
expect(error).toHaveBeenCalledWith(res, 422, 'Unprocessable Entity', 'Failed to send the email, please try again.');
537+
const clientMessage = error.mock.calls[0][3];
538+
expect(clientMessage).not.toContain('sk_live_abc123');
539+
expect(clientMessage).not.toContain('Resend API error');
540+
541+
// Real error still logged server-side with context, so it is not lost.
542+
expect(mockError).toHaveBeenCalledWith('[auth.resendVerification] failed', expect.objectContaining({
543+
userId: 'u1',
544+
message: providerError.message,
545+
}));
546+
});
547+
});

modules/invitations/controllers/invitations.controller.js

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
*/
44
import errors from '../../../lib/helpers/errors.js';
55
import responses from '../../../lib/helpers/responses.js';
6+
import logger from '../../../lib/services/logger.js';
67
import InvitationService from '../services/invitations.service.js';
78

89
/**
@@ -83,10 +84,24 @@ const resend = async (req, res) => {
8384
responses.success(res, 'invitation resent')(invitation);
8485
} catch (err) {
8586
// Thread ANY service-thrown AppError status (409 duplicate-pending, 404
86-
// unknown id, etc.) rather than flattening everything but 409 to 422 — a
87-
// 404 must not surface as Unprocessable Entity if a future caller reaches
88-
// the service without the invitationByID param-loader's 404 guard.
89-
const status = err.status ?? 422;
87+
// unknown id, 422 mailer-not-configured, etc.) rather than flattening
88+
// everything but 409 to 422 — a 404 must not surface as Unprocessable
89+
// Entity if a future caller reaches the service without the
90+
// invitationByID param-loader's 404 guard.
91+
// #3966 hardening: InvitationService.resend's mails.sendMail(...) call is
92+
// unguarded and now propagates (#3966) — a raw transport rejection has no
93+
// `.status` (unlike the AppErrors thrown deliberately above) and must not
94+
// leak the raw SMTP/provider error string to the client. Log the real
95+
// error server-side with context; respond with a stable generic message.
96+
if (err.status == null) {
97+
logger.error('[invitations.resend] failed', {
98+
invitationId: req.invitation?.id,
99+
message: err?.message,
100+
stack: err?.stack,
101+
});
102+
return responses.error(res, 422, 'Unprocessable Entity', 'Failed to send the email, please try again.')(err);
103+
}
104+
const status = err.status;
90105
const title = status === 409 ? 'Conflict' : status === 404 ? 'Not Found' : 'Unprocessable Entity';
91106
responses.error(res, status, title, errors.getMessage(err))(err);
92107
}

modules/invitations/tests/invitations.controller.unit.tests.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@ const successInner = jest.fn();
1313
const errorInner = jest.fn();
1414
const success = jest.fn(() => successInner);
1515
const error = jest.fn(() => errorInner);
16+
const mockLogger = { error: jest.fn(), warn: jest.fn(), info: jest.fn() };
1617

1718
jest.unstable_mockModule('../services/invitations.service.js', () => ({ default: mockService }));
1819
jest.unstable_mockModule('../../../lib/helpers/responses.js', () => ({ default: { success, error } }));
1920
jest.unstable_mockModule('../../../lib/helpers/errors.js', () => ({ default: { getMessage: jest.fn((e) => e?.message || 'err') } }));
21+
jest.unstable_mockModule('../../../lib/services/logger.js', () => ({ default: mockLogger }));
2022

2123
const controller = (await import('../controllers/invitations.controller.js')).default;
2224

@@ -94,6 +96,26 @@ describe('invitations.controller.resend', () => {
9496
await controller.resend({ invitation: { id: 'i1' } }, makeRes());
9597
expect(error).toHaveBeenCalledWith(expect.anything(), 422, 'Unprocessable Entity', expect.any(String));
9698
});
99+
// #3966 hardening: a bare transport rejection (no `.status`, unlike the
100+
// deliberate AppErrors above) must not leak the raw provider/SMTP error
101+
// string to the client — only a stable generic message — while the real
102+
// error still gets logged server-side with context.
103+
test('mail-transport failure (no .status) responds with a generic message, never the raw provider error, and logs server-side', async () => {
104+
const providerError = new Error('Resend API error: 401 Unauthorized — invalid API key sk_live_abc123');
105+
mockService.resend.mockRejectedValue(providerError);
106+
await controller.resend({ invitation: { id: 'i1' } }, makeRes());
107+
108+
expect(error).toHaveBeenCalledWith(expect.anything(), 422, 'Unprocessable Entity', expect.any(String));
109+
const clientMessage = error.mock.calls[0][3];
110+
expect(clientMessage).not.toContain('sk_live_abc123');
111+
expect(clientMessage).not.toContain('Resend API error');
112+
expect(clientMessage).toBe('Failed to send the email, please try again.');
113+
114+
expect(mockLogger.error).toHaveBeenCalledWith('[invitations.resend] failed', expect.objectContaining({
115+
invitationId: 'i1',
116+
message: providerError.message,
117+
}));
118+
});
97119
});
98120

99121
describe('invitations.controller.verify', () => {

0 commit comments

Comments
 (0)