feat: Add conditional MAIL_HOST validation, lazy-load mailer, and update Gitea workflow to use vars for mail and Sentry environment variables.

2026-02-05 12:39:51 +01:00
parent e8957e0672
commit db4cf354ff
5 changed files with 199 additions and 55 deletions
--- a/.gitea/workflows/deploy.yml
+++ b/.gitea/workflows/deploy.yml
@@ -296,13 +296,13 @@ jobs:
      NEXT_PUBLIC_BASE_URL:          ${{ needs.prepare.outputs.next_public_base_url }}
      NEXT_PUBLIC_UMAMI_WEBSITE_ID:  ${{ needs.prepare.outputs.target == 'production' && secrets.NEXT_PUBLIC_UMAMI_WEBSITE_ID  || (needs.prepare.outputs.target == 'staging' && secrets.STAGING_NEXT_PUBLIC_UMAMI_WEBSITE_ID  || secrets.TESTING_NEXT_PUBLIC_UMAMI_WEBSITE_ID  || secrets.NEXT_PUBLIC_UMAMI_WEBSITE_ID) }}
      NEXT_PUBLIC_UMAMI_SCRIPT_URL:  ${{ needs.prepare.outputs.target == 'production' && secrets.NEXT_PUBLIC_UMAMI_SCRIPT_URL  || (needs.prepare.outputs.target == 'staging' && secrets.STAGING_NEXT_PUBLIC_UMAMI_SCRIPT_URL  || secrets.TESTING_NEXT_PUBLIC_UMAMI_SCRIPT_URL  || secrets.NEXT_PUBLIC_UMAMI_SCRIPT_URL) }}
-      SENTRY_DSN:                    ${{ needs.prepare.outputs.target == 'production' && secrets.SENTRY_DSN                    || (needs.prepare.outputs.target == 'staging' && secrets.STAGING_SENTRY_DSN                    || secrets.TESTING_SENTRY_DSN                    || secrets.SENTRY_DSN) }}
-      MAIL_HOST:                     ${{ secrets.MAIL_HOST }}
-      MAIL_PORT:                     ${{ secrets.MAIL_PORT }}
-      MAIL_USERNAME:                 ${{ secrets.MAIL_USERNAME }}
+      SENTRY_DSN:                    ${{ secrets.SENTRY_DSN                    || vars.SENTRY_DSN                    || (needs.prepare.outputs.target == 'production' && secrets.SENTRY_DSN                    || (needs.prepare.outputs.target == 'staging' && secrets.STAGING_SENTRY_DSN                    || secrets.TESTING_SENTRY_DSN                    || secrets.SENTRY_DSN)) }}
+      MAIL_HOST:                     ${{ secrets.MAIL_HOST || vars.MAIL_HOST }}
+      MAIL_PORT:                     ${{ secrets.MAIL_PORT || vars.MAIL_PORT }}
+      MAIL_USERNAME:                 ${{ secrets.MAIL_USERNAME || vars.MAIL_USERNAME }}
      MAIL_PASSWORD:                 ${{ secrets.MAIL_PASSWORD }}
-      MAIL_FROM:                     ${{ secrets.MAIL_FROM }}
-      MAIL_RECIPIENTS:               ${{ secrets.MAIL_RECIPIENTS }}
+      MAIL_FROM:                     ${{ secrets.MAIL_FROM || vars.MAIL_FROM }}
+      MAIL_RECIPIENTS:               ${{ secrets.MAIL_RECIPIENTS || vars.MAIL_RECIPIENTS }}
      DIRECTUS_URL:                  ${{ needs.prepare.outputs.directus_url }}
      DIRECTUS_HOST:                 ${{ needs.prepare.outputs.directus_host }}
      PROJECT_NAME:                  ${{ needs.prepare.outputs.project_name }}
--- a/lib/env.test.ts
+++ b/lib/env.test.ts
@@ -0,0 +1,60 @@
+import { describe, it, expect } from 'vitest';
+import { envSchema } from './env';
+
+describe('envSchema', () => {
+  it('should allow missing MAIL_HOST in development', () => {
+    const result = envSchema.safeParse({
+      NEXT_PUBLIC_BASE_URL: 'http://localhost:3000',
+      TARGET: 'development',
+    });
+    expect(result.success).toBe(true);
+  });
+
+  it('should require MAIL_HOST in production', () => {
+    const result = envSchema.safeParse({
+      NEXT_PUBLIC_BASE_URL: 'https://example.com',
+      TARGET: 'production',
+    });
+    expect(result.success).toBe(false);
+    if (!result.success) {
+      expect(result.error.issues[0].message).toBe(
+        'MAIL_HOST is required in non-development environments',
+      );
+    }
+  });
+
+  it('should require MAIL_HOST in testing', () => {
+    const result = envSchema.safeParse({
+      NEXT_PUBLIC_BASE_URL: 'https://testing.example.com',
+      TARGET: 'testing',
+    });
+    expect(result.success).toBe(false);
+    if (!result.success) {
+      expect(result.error.issues[0].message).toBe(
+        'MAIL_HOST is required in non-development environments',
+      );
+    }
+  });
+
+  it('should require MAIL_HOST in staging', () => {
+    const result = envSchema.safeParse({
+      NEXT_PUBLIC_BASE_URL: 'https://staging.example.com',
+      TARGET: 'staging',
+    });
+    expect(result.success).toBe(false);
+    if (!result.success) {
+      expect(result.error.issues[0].message).toBe(
+        'MAIL_HOST is required in non-development environments',
+      );
+    }
+  });
+
+  it('should pass if MAIL_HOST is provided in production', () => {
+    const result = envSchema.safeParse({
+      NEXT_PUBLIC_BASE_URL: 'https://example.com',
+      TARGET: 'production',
+      MAIL_HOST: 'smtp.example.com',
+    });
+    expect(result.success).toBe(true);
+  });
+});
--- a/lib/env.ts
+++ b/lib/env.ts
@@ -8,51 +8,64 @@ const preprocessEmptyString = (val: unknown) => (val === '' ? undefined : val);
 /**
 * Environment variable schema.
 */
-export const envSchema = z.object({
-  NODE_ENV: z.enum(['development', 'production', 'test']).default('development'),
-  NEXT_PUBLIC_BASE_URL: z.preprocess(preprocessEmptyString, z.string().url()),
-  NEXT_PUBLIC_TARGET: z.enum(['development', 'testing', 'staging', 'production']).optional(),
+export const envSchema = z
+  .object({
+    NODE_ENV: z.enum(['development', 'production', 'test']).default('development'),
+    NEXT_PUBLIC_BASE_URL: z.preprocess(preprocessEmptyString, z.string().url()),
+    NEXT_PUBLIC_TARGET: z.enum(['development', 'testing', 'staging', 'production']).optional(),

-  // Analytics
-  NEXT_PUBLIC_UMAMI_WEBSITE_ID: z.preprocess(preprocessEmptyString, z.string().optional()),
-  NEXT_PUBLIC_UMAMI_SCRIPT_URL: z.preprocess(
-    preprocessEmptyString,
-    z.string().url().default('https://analytics.infra.mintel.me/script.js'),
-  ),
+    // Analytics
+    NEXT_PUBLIC_UMAMI_WEBSITE_ID: z.preprocess(preprocessEmptyString, z.string().optional()),
+    NEXT_PUBLIC_UMAMI_SCRIPT_URL: z.preprocess(
+      preprocessEmptyString,
+      z.string().url().default('https://analytics.infra.mintel.me/script.js'),
+    ),

-  // Error Tracking
-  SENTRY_DSN: z.preprocess(preprocessEmptyString, z.string().optional()),
+    // Error Tracking
+    SENTRY_DSN: z.preprocess(preprocessEmptyString, z.string().optional()),

-  // Logging
-  LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'),
+    // Logging
+    LOG_LEVEL: z.enum(['debug', 'info', 'warn', 'error']).default('info'),

-  // Mail
-  MAIL_HOST: z.preprocess(preprocessEmptyString, z.string().optional()),
-  MAIL_PORT: z.preprocess(preprocessEmptyString, z.coerce.number().default(587)),
-  MAIL_USERNAME: z.preprocess(preprocessEmptyString, z.string().optional()),
-  MAIL_PASSWORD: z.preprocess(preprocessEmptyString, z.string().optional()),
-  MAIL_FROM: z.preprocess(preprocessEmptyString, z.string().optional()),
-  MAIL_RECIPIENTS: z.preprocess(
-    (val) => (typeof val === 'string' ? val.split(',').filter(Boolean) : val),
-    z.array(z.string()).default([]),
-  ),
+    // Mail
+    MAIL_HOST: z.preprocess(preprocessEmptyString, z.string().optional()),
+    MAIL_PORT: z.preprocess(preprocessEmptyString, z.coerce.number().default(587)),
+    MAIL_USERNAME: z.preprocess(preprocessEmptyString, z.string().optional()),
+    MAIL_PASSWORD: z.preprocess(preprocessEmptyString, z.string().optional()),
+    MAIL_FROM: z.preprocess(preprocessEmptyString, z.string().optional()),
+    MAIL_RECIPIENTS: z.preprocess(
+      (val) => (typeof val === 'string' ? val.split(',').filter(Boolean) : val),
+      z.array(z.string()).default([]),
+    ),

-  // Directus
-  DIRECTUS_URL: z.preprocess(
-    preprocessEmptyString,
-    z.string().url().default('http://localhost:8055'),
-  ),
-  DIRECTUS_ADMIN_EMAIL: z.preprocess(preprocessEmptyString, z.string().optional()),
-  DIRECTUS_ADMIN_PASSWORD: z.preprocess(preprocessEmptyString, z.string().optional()),
-  DIRECTUS_API_TOKEN: z.preprocess(preprocessEmptyString, z.string().optional()),
-  INTERNAL_DIRECTUS_URL: z.preprocess(preprocessEmptyString, z.string().url().optional()),
+    // Directus
+    DIRECTUS_URL: z.preprocess(
+      preprocessEmptyString,
+      z.string().url().default('http://localhost:8055'),
+    ),
+    DIRECTUS_ADMIN_EMAIL: z.preprocess(preprocessEmptyString, z.string().optional()),
+    DIRECTUS_ADMIN_PASSWORD: z.preprocess(preprocessEmptyString, z.string().optional()),
+    DIRECTUS_API_TOKEN: z.preprocess(preprocessEmptyString, z.string().optional()),
+    INTERNAL_DIRECTUS_URL: z.preprocess(preprocessEmptyString, z.string().url().optional()),

-  // Deploy Target
-  TARGET: z.enum(['development', 'testing', 'staging', 'production']).optional(),
-  // Gotify
-  GOTIFY_URL: z.preprocess(preprocessEmptyString, z.string().url().optional()),
-  GOTIFY_TOKEN: z.preprocess(preprocessEmptyString, z.string().optional()),
-});
+    // Deploy Target
+    TARGET: z.enum(['development', 'testing', 'staging', 'production']).optional(),
+    // Gotify
+    GOTIFY_URL: z.preprocess(preprocessEmptyString, z.string().url().optional()),
+    GOTIFY_TOKEN: z.preprocess(preprocessEmptyString, z.string().optional()),
+  })
+  .superRefine((data, ctx) => {
+    const target = data.NEXT_PUBLIC_TARGET || data.TARGET;
+    const isDev = target === 'development' || !target;
+
+    if (!isDev && !data.MAIL_HOST) {
+      ctx.addIssue({
+        code: z.ZodIssueCode.custom,
+        message: 'MAIL_HOST is required in non-development environments',
+        path: ['MAIL_HOST'],
+      });
+    }
+  });

 export type Env = z.infer<typeof envSchema>;

--- a/lib/mail/mailer.test.ts
+++ b/lib/mail/mailer.test.ts
@@ -0,0 +1,59 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+import { sendEmail } from './mailer';
+import { config } from '../config';
+
+// Mock getServerAppServices to avoid full app initialization
+vi.mock('@/lib/services/create-services.server', () => ({
+  getServerAppServices: () => ({
+    logger: {
+      child: () => ({
+        info: vi.fn(),
+        error: vi.fn(),
+        warn: vi.fn(),
+      }),
+    },
+  }),
+}));
+
+// Mock config
+vi.mock('../config', () => ({
+  config: {
+    mail: {
+      host: 'smtp.example.com',
+      port: 587,
+      user: 'user',
+      pass: 'pass',
+      from: 'from@example.com',
+      recipients: ['to@example.com'],
+    },
+  },
+  getConfig: vi.fn(),
+}));
+
+describe('mailer', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  describe('sendEmail', () => {
+    it('should throw error if MAIL_HOST is missing', async () => {
+      // Temporarily nullify host
+      const originalHost = config.mail.host;
+      (config.mail as any).host = '';
+
+      const result = await sendEmail({
+        subject: 'Test',
+        html: '<p>Test</p>',
+      });
+
+      expect(result.success).toBe(false);
+      expect((result.error as Error).message).toContain('MAIL_HOST is not configured');
+
+      // Restore host
+      (config.mail as any).host = originalHost;
+    });
+
+    // In a real environment, we'd mock nodemailer, but for now we focus on the validation logic
+    // we added. Full SMTP integration tests are usually out of scope for unit tests.
+  });
+});
--- a/lib/mail/mailer.ts
+++ b/lib/mail/mailer.ts
@@ -3,15 +3,27 @@ import { getServerAppServices } from '@/lib/services/create-services.server';
 import { config } from '../config';
 import { ReactElement } from 'react';

-const transporter = nodemailer.createTransport({
-  host: config.mail.host,
-  port: config.mail.port,
-  secure: config.mail.port === 465,
-  auth: {
-    user: config.mail.user,
-    pass: config.mail.pass,
-  },
-});
+let transporterInstance: nodemailer.Transporter | null = null;
+
+function getTransporter() {
+  if (transporterInstance) return transporterInstance;
+
+  if (!config.mail.host) {
+    throw new Error('MAIL_HOST is not configured. Please check your environment variables.');
+  }
+
+  transporterInstance = nodemailer.createTransport({
+    host: config.mail.host,
+    port: config.mail.port,
+    secure: config.mail.port === 465,
+    auth: {
+      user: config.mail.user,
+      pass: config.mail.pass,
+    },
+  });
+
+  return transporterInstance;
+}

 interface SendEmailOptions {
  to?: string | string[];
@@ -32,7 +44,7 @@ export async function sendEmail({ to, subject, html }: SendEmailOptions) {
  const logger = getServerAppServices().logger.child({ component: 'mailer' });

  try {
-    const info = await transporter.sendMail(mailOptions);
+    const info = await getTransporter().sendMail(mailOptions);
    logger.info('Email sent successfully', { messageId: info.messageId, subject, recipients });
    return { success: true, messageId: info.messageId };
  } catch (error) {