fix(security): centralize SMTP transport creation

Centralize SMTP transport creation to reduce the duplicated CodeQL SMTP request-forgery path.
This commit is contained in:
Daniel Volz
2026-04-08 20:00:57 +02:00
committed by GitHub
parent aa050f7dc5
commit 002f16c505
3 changed files with 41 additions and 55 deletions
+4 -16
View File
@@ -1,6 +1,5 @@
import { and, eq } from "drizzle-orm";
import type { FastifyInstance, FastifyRequest } from "fastify";
import nodemailer from "nodemailer";
import { db } from "../db/client.js";
import { medications } from "../db/schema.js";
import {
@@ -20,7 +19,7 @@ import {
type StockReminderItem as SharedStockReminderItem,
} from "../services/notifications/builders.js";
import { getSmtpConfig, sendEmailNotification, sendPushNotification } from "../services/notifications/delivery.js";
import { escapeHtml, getDeliveryError, getPlannerUnit, isContainerPackage } from "../services/planner-service.js";
import { escapeHtml, getPlannerUnit, isContainerPackage } from "../services/planner-service.js";
import { updateReminderSentTime, updateUserReminderSentTime } from "../services/reminder-scheduler.js";
import type { AuthUser } from "../types/fastify.js";
import {
@@ -428,19 +427,9 @@ ${getFooterPlain(language)}`;
`;
try {
const transporter = nodemailer.createTransport({
host: smtpHost,
port: smtpPort,
secure: smtpSecure,
auth: {
user: smtpUser,
pass: smtpPass ?? "",
},
});
request.log.info({ userId, recipientEmail: email }, "[Planner] Sending demand email");
const mailResult = await transporter.sendMail({
const mailResult = await sendEmailNotification({
from: smtpFrom,
to: email,
subject: t(dc.subject, { from: fromDate, until: untilDate }),
@@ -448,9 +437,8 @@ ${getFooterPlain(language)}`;
html,
});
const deliveryError = getDeliveryError(mailResult);
if (deliveryError) {
throw new Error(deliveryError);
if (!mailResult.success) {
throw new Error(mailResult.error ?? "Failed to send demand email");
}
request.log.info(
+14 -30
View File
@@ -1,10 +1,10 @@
import { eq } from "drizzle-orm";
import type { FastifyInstance, FastifyReply, FastifyRequest } from "fastify";
import nodemailer from "nodemailer";
import { db } from "../db/client.js";
import { userSettings } from "../db/schema.js";
import { getAnonymousUserId, requireAuth } from "../plugins/auth.js";
import { env } from "../plugins/env.js";
import { getSmtpConfig, sendEmailNotification } from "../services/notifications/delivery.js";
import {
classifyTestEmailFailure,
getAllUserSettingsFromDb,
@@ -445,49 +445,34 @@ export async function settingsRoutes(app: FastifyInstance) {
async (request, reply) => {
const { email } = request.body;
const smtpHost = process.env.SMTP_HOST;
const smtpUser = process.env.SMTP_USER;
const smtpPass = process.env.SMTP_TOKEN || process.env.SMTP_PASS;
const smtpPort = parseInt(process.env.SMTP_PORT ?? "587", 10);
const smtpSecure = process.env.SMTP_SECURE === "true";
const smtpFrom = process.env.SMTP_FROM ?? smtpUser;
const smtp = getSmtpConfig();
request.log.info(
{
to: email,
hasSmtpHost: Boolean(smtpHost),
hasSmtpUser: Boolean(smtpUser),
hasSmtpPass: Boolean(smtpPass),
hasSmtpFrom: Boolean(smtpFrom),
smtpPort,
smtpSecure,
hasSmtpHost: Boolean(smtp.host),
hasSmtpUser: Boolean(smtp.user),
hasSmtpPass: Boolean(smtp.pass),
hasSmtpFrom: Boolean(smtp.from),
smtpPort: smtp.port,
smtpSecure: smtp.secure,
},
"[Settings] Test email request received"
);
if (!smtpHost || !smtpUser) {
if (!smtp.host || !smtp.user) {
request.log.warn(
{ to: email, hasSmtpHost: Boolean(smtpHost), hasSmtpUser: Boolean(smtpUser) },
{ to: email, hasSmtpHost: Boolean(smtp.host), hasSmtpUser: Boolean(smtp.user) },
"[Settings] Test email skipped: SMTP not configured"
);
return reply.status(400).send({ error: "SMTP not configured" });
}
try {
const transporter = nodemailer.createTransport({
host: smtpHost,
port: smtpPort,
secure: smtpSecure,
auth: {
user: smtpUser,
pass: smtpPass ?? "",
},
});
request.log.info({ to: email }, "[Settings] Sending test email");
const mailResult = await transporter.sendMail({
from: smtpFrom,
const mailResult = await sendEmailNotification({
from: smtp.from,
to: email,
subject: "MedAssist-ng - Test Email",
text: "This is a test email from MedAssist-ng. If you received this, your email configuration is working correctly!",
@@ -502,9 +487,8 @@ export async function settingsRoutes(app: FastifyInstance) {
`,
});
const deliveryError = getDeliveryError(mailResult);
if (deliveryError) {
throw new Error(deliveryError);
if (!mailResult.success) {
throw new Error(mailResult.error ?? "Failed to send test email");
}
request.log.info({ to: email, messageId: mailResult.messageId }, "[Settings] Test email sent");
+23 -9
View File
@@ -64,6 +64,25 @@ export function getSmtpConfig(): {
return { host, user, pass, port, secure, from };
}
export function createSmtpTransport(smtp = getSmtpConfig()) {
if (!smtp.host || !smtp.user) {
return null;
}
// The SMTP endpoint is configured by the server operator via environment variables,
// not derived from request-controlled input.
// lgtm [js/request-forgery]
return nodemailer.createTransport({
host: smtp.host,
port: smtp.port,
secure: smtp.secure,
auth: {
user: smtp.user,
pass: smtp.pass ?? "",
},
});
}
export async function sendEmailNotification(input: EmailDeliveryRequest): Promise<EmailDeliveryResult> {
const smtp = getSmtpConfig();
if (!smtp.host || !smtp.user) {
@@ -71,15 +90,10 @@ export async function sendEmailNotification(input: EmailDeliveryRequest): Promis
}
try {
const transporter = nodemailer.createTransport({
host: smtp.host,
port: smtp.port,
secure: smtp.secure,
auth: {
user: smtp.user,
pass: smtp.pass ?? "",
},
});
const transporter = createSmtpTransport(smtp);
if (!transporter) {
return { success: false, error: "SMTP not configured" };
}
const mailResult = await transporter.sendMail({
from: input.from ?? smtp.from,