diff --git a/backend/src/routes/adminCustomers.js b/backend/src/routes/adminCustomers.js index 0d3c81e7..cc03115c 100644 --- a/backend/src/routes/adminCustomers.js +++ b/backend/src/routes/adminCustomers.js @@ -165,7 +165,7 @@ router.post('/invite', [ body('prefill.postal_code').optional({ nullable: true }).isString().isLength({ max: 20 }), body('prefill.city').optional({ nullable: true }).isString().isLength({ max: 120 }), body('prefill.state').optional({ nullable: true }).isString().isLength({ max: 120 }), - body('prefill.country_code').optional({ nullable: true }).isString().isLength({ max: 2 }), + body('prefill.country_code').optional({ values: 'falsy' }).isLength({ min: 2, max: 2 }).isAlpha().withMessage('country_code must be a 2-letter ISO code').customSanitizer((v) => (v || '').toUpperCase()), // Per-customer preferred language. Drives portal UI + quote/invoice // PDF locale. Defaults at insert time to the business profile's // default_locale when the admin doesn't supply one (see @@ -246,7 +246,7 @@ router.post('/', [ body('prefill.postal_code').optional({ nullable: true }).isString().isLength({ max: 20 }), body('prefill.city').optional({ nullable: true }).isString().isLength({ max: 120 }), body('prefill.state').optional({ nullable: true }).isString().isLength({ max: 120 }), - body('prefill.country_code').optional({ nullable: true }).isString().isLength({ max: 2 }), + body('prefill.country_code').optional({ values: 'falsy' }).isLength({ min: 2, max: 2 }).isAlpha().withMessage('country_code must be a 2-letter ISO code').customSanitizer((v) => (v || '').toUpperCase()), body('prefill.country_name').optional({ nullable: true }).isString().isLength({ max: 120 }), body('prefill.preferred_language').optional({ nullable: true }).isString().isLength({ min: 2, max: 8 }), // At least one human-readable identifier so the record isn't a @@ -379,7 +379,7 @@ router.put('/:id', [ body('postal_code').optional({ nullable: true }).isString().isLength({ max: 20 }), body('city').optional({ nullable: true }).isString().isLength({ max: 120 }), body('state').optional({ nullable: true }).isString().isLength({ max: 120 }), - body('country_code').optional({ nullable: true }).isString().isLength({ max: 2 }), + body('country_code').optional({ values: 'falsy' }).isLength({ min: 2, max: 2 }).isAlpha().withMessage('country_code must be a 2-letter ISO code').customSanitizer((v) => (v || '').toUpperCase()), body('country_name').optional({ nullable: true }).isString().isLength({ max: 120 }), body('preferred_language').optional({ nullable: true }).isString().isLength({ max: 8 }), body('notes').optional({ nullable: true }).isString(), diff --git a/backend/src/routes/adminInvoices.js b/backend/src/routes/adminInvoices.js index b114bf44..23d8f2bf 100644 --- a/backend/src/routes/adminInvoices.js +++ b/backend/src/routes/adminInvoices.js @@ -32,6 +32,23 @@ const { db } = require('../database/db'); const router = express.Router(); +// PR #603 review follow-up #2 — bound payment dates. `isISO8601()` alone +// accepts year 1900/9999; cash-basis revenue keys on paid_at, so a typo +// (2026→2226) would silently push a payment out of every dashboard window +// forever. Reject anything before 2000-01-01 or more than 30 days in the +// future (small future window covers value-date lag without allowing fat- +// finger years). Use as `.custom(isReasonablePaidAt)` after `.isISO8601()`. +function isReasonablePaidAt(value) { + const d = new Date(value); + if (Number.isNaN(d.getTime())) throw new Error('Invalid payment date'); + const min = new Date('2000-01-01T00:00:00Z'); + const max = new Date(Date.now() + 30 * 24 * 60 * 60 * 1000); + if (d < min || d > max) { + throw new Error('Payment date must be between 2000-01-01 and 30 days from now'); + } + return true; +} + // Multer config for "import historical invoice" PDF uploads. Stored // under storage/business-docs/invoice-imports// so // imported files don't collide with the renderer's own output under @@ -438,7 +455,7 @@ router.post( body('currency').optional({ values: 'falsy' }).isString().isLength({ min: 3, max: 3 }), body('status').optional({ values: 'falsy' }).isIn(['sent', 'paid', 'overdue']), body('paidAmountMinor').optional({ values: 'falsy' }).isInt({ min: 0 }), - body('paidAt').optional({ values: 'falsy' }).isISO8601(), + body('paidAt').optional({ values: 'falsy' }).isISO8601().custom(isReasonablePaidAt), body('language').optional({ values: 'falsy' }).isString().isLength({ max: 8 }), ], handleAsync(async (req, res) => { @@ -745,7 +762,7 @@ router.post( [ param('id').isInt({ min: 1 }), body('amountMinor').isInt({ min: 1 }), - body('paidAt').optional({ values: 'falsy' }).isISO8601(), + body('paidAt').optional({ values: 'falsy' }).isISO8601().custom(isReasonablePaidAt), body('paymentMethod').optional({ values: 'falsy' }).isString().isLength({ max: 64 }), body('reference').optional({ values: 'falsy' }).isString().isLength({ max: 128 }), body('notes').optional({ values: 'falsy' }).isString().isLength({ max: 5000 }), diff --git a/backend/src/services/emailProcessor.js b/backend/src/services/emailProcessor.js index 0e9b2412..58462bfa 100644 --- a/backend/src/services/emailProcessor.js +++ b/backend/src/services/emailProcessor.js @@ -38,6 +38,13 @@ async function initializeTransporter(forceReinit = false) { // Configuration has changed or first initialization logger.info('Initializing email transporter' + (lastConfigHash && currentConfigHash !== lastConfigHash ? ' (configuration changed)' : '')); + // PR #603 review follow-up #3 — release the previous transporter before + // swapping it. Harmless today (no connection pool), but prevents a + // socket/connection leak if `pool: true` is ever enabled on the transport. + if (transporter && typeof transporter.close === 'function') { + try { transporter.close(); } catch (_) { /* best-effort */ } + } + transporter = nodemailer.createTransport({ host: config.smtp_host, port: config.smtp_port, @@ -252,6 +259,19 @@ async function wrapEmailHtml(htmlBody, subject, language = 'en') { const logoFullUrl = `${frontendUrl}${logoPath.startsWith('/') ? '' : '/'}${logoPath}`; logger.debug('Email logo URL:', { frontendUrl, logoPath, logoFullUrl }); + const year = new Date().getFullYear(); + // PR review follow-up — Outlook (Word engine) and Apple Mail under some + // configs STRIP the - - + + + + + + `; } @@ -886,7 +916,18 @@ async function getScheduledEmailConfig() { const schedule = normaliseSchedule(profile.business_hours); let timezone = (profile.timezone || '').trim(); - if (!timezone) timezone = Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC'; + if (!timezone) { + // PR #603 review follow-up #4 — business hours are configured but the + // profile timezone is blank, so we fall back to the SERVER's tz (usually + // UTC on a Docker host). That silently shifts every business-hours + // calculation. Warn loudly so the admin sets business_profile.timezone. + timezone = Intl.DateTimeFormat().resolvedOptions().timeZone || 'UTC'; + logger.warn( + 'Scheduled-email business hours are set but business_profile.timezone is blank — ' + + `falling back to the server timezone (${timezone}). Set the profile timezone ` + + 'so business-hours snapping uses your local time, not the server\'s.', + ); + } // Reject a bogus tz before it reaches Intl in the snap helper. try { new Intl.DateTimeFormat('en-US', { timeZone: timezone }); diff --git a/backend/src/services/invoiceService.js b/backend/src/services/invoiceService.js index 91a92e68..4f708097 100644 --- a/backend/src/services/invoiceService.js +++ b/backend/src/services/invoiceService.js @@ -694,6 +694,22 @@ async function createInvoice(payload, adminId, trx = db) { const customer = await trx('customer_accounts').where({ id: payload.customerAccountId }).first(); ensureCustomerCanBill(customer); + // PR #603 review follow-up #1 — when an invoice is attached to an event, + // make sure that event actually belongs to the chosen customer. Without + // this, a typo'd/copy-pasted eventId silently links the invoice to an + // unrelated event, producing misleading reporting links. Only enforced + // when the event HAS customer assignments (an event with none — e.g. a + // legacy import — is allowed through, since we can't prove a mismatch). + if (payload.eventId && await trx.schema.hasTable('event_customer_assignments')) { + const assignments = await trx('event_customer_assignments') + .where({ event_id: payload.eventId }) + .select('customer_account_id'); + if (assignments.length > 0 && + !assignments.some(a => a.customer_account_id === payload.customerAccountId)) { + throw new AppError('The selected event is not assigned to this customer', 422, 'EVENT_CUSTOMER_MISMATCH'); + } + } + // Accumulator intercept (migration 128). For customers in // billing_cadence='monthly' OR 'manual' mode every createInvoice call // APPENDS line items onto a single running draft instead of minting a