fix(messages): PR #769 review — escape reply sender (XSS), gate backend routes, exact customer match

- BLOCKER: stored XSS via inbound sender display name. The reply stub built raw
  HTML with the unsanitized From name and set it as innerHTML on the composer's
  contentEditable (admin origin) → onerror JS ran on Reply. Now HTML-escape
  from_address in the stub AND DOMPurify-sanitize the composer body before
  innerHTML (defense in depth).
- Gate the NEW Messages routes with requireFeatureFlag('messaging') per-route
  (queue/:id, received/:id, item/*, identities, accounts, accounts/test, send)
  — NOT the shared /email mount, so the pre-existing email-config endpoints stay
  ungated.
- DocumentActionModal auto-picks a customer only on an EXACT email match
  (customer search is prefix/fuzzy), else leaves the picker to the admin.
This commit is contained in:
Luca
2026-07-07 18:28:26 +02:00
parent 99d5996561
commit bb235e72e5
4 changed files with 33 additions and 12 deletions
+13 -9
View File
@@ -4,6 +4,10 @@ const { body, query, validationResult } = require('express-validator');
const { db, logActivity } = require('../database/db');
const { adminAuth } = require('../middleware/auth');
const { requirePermission } = require('../middleware/permissions');
// Gate the NEW Messages routes on the `messaging` flag (per-route, NOT the whole
// /email mount — the pre-existing config/queue/received endpoints stay ungated).
const { requireFeatureFlag } = require('../middleware/requireFeatureFlag');
const messagingGate = requireFeatureFlag('messaging');
const { wrapEmailHtml, processEmailQueue } = require('../services/emailProcessor');
const { errorResponse } = require('../utils/routeHelpers');
const logger = require('../utils/logger');
@@ -287,7 +291,7 @@ router.get('/received', adminAuth, requirePermission('email.view'), async (req,
// Single received email WITH its captured (server-sanitized) body — Messages
// reading pane. body_html was already sanitized on ingest; the viewer renders
// it in a script-less sandboxed iframe as well.
router.get('/received/:id', adminAuth, requirePermission('email.view'), async (req, res) => {
router.get('/received/:id', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const id = parseInt(req.params.id, 10);
if (!Number.isInteger(id)) return res.status(400).json({ error: 'Invalid id' });
@@ -302,7 +306,7 @@ router.get('/received/:id', adminAuth, requirePermission('email.view'), async (r
// Move an email between mailbox states: Archive / Delete (soft) or Restore
// (back to active). kind = 'queue' | 'received'. Delete is a soft move to the
// trash; the row is only removed for good by the DELETE handler below.
router.post('/item/:kind/:id/state', adminAuth, requirePermission('email.view'), async (req, res) => {
router.post('/item/:kind/:id/state', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const table = req.params.kind === 'received' ? 'received_emails' : req.params.kind === 'queue' ? 'email_queue' : null;
if (!table) return res.status(400).json({ error: 'Invalid kind' });
@@ -319,7 +323,7 @@ router.post('/item/:kind/:id/state', adminAuth, requirePermission('email.view'),
});
// Permanently delete an email row — only offered from the Deleted folder.
router.delete('/item/:kind/:id', adminAuth, requirePermission('email.edit'), async (req, res) => {
router.delete('/item/:kind/:id', adminAuth, messagingGate, requirePermission('email.edit'), async (req, res) => {
try {
const table = req.params.kind === 'received' ? 'received_emails' : req.params.kind === 'queue' ? 'email_queue' : null;
if (!table) return res.status(400).json({ error: 'Invalid kind' });
@@ -334,7 +338,7 @@ router.delete('/item/:kind/:id', adminAuth, requirePermission('email.edit'), asy
// Additional inbound mailboxes (beyond the primary accounting IMAP in
// email_configs) — e.g. the customer hello@ box. Passwords are masked out.
router.get('/accounts', adminAuth, requirePermission('email.view'), async (req, res) => {
router.get('/accounts', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const rows = await db('mail_accounts').orderBy('id');
res.json({ items: rows.map((a) => ({
@@ -351,7 +355,7 @@ router.get('/accounts', adminAuth, requirePermission('email.view'), async (req,
// the REAL configured addresses instead of hardcoded placeholders. Accounting =
// the primary IMAP login (rechnungen@); customers = the hello@ mailbox; the
// automated stream sends from the global SMTP from-address.
router.get('/identities', adminAuth, requirePermission('email.view'), async (req, res) => {
router.get('/identities', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const cfg = await db('email_configs').first();
let customers = null;
@@ -371,7 +375,7 @@ router.get('/identities', adminAuth, requirePermission('email.view'), async (req
// Upsert a mailbox by account_key. A masked password ('********') keeps the
// stored value so the admin never has to re-type it.
router.post('/accounts', adminAuth, requirePermission('email.edit'), async (req, res) => {
router.post('/accounts', adminAuth, messagingGate, requirePermission('email.edit'), async (req, res) => {
try {
const b = req.body || {};
if (!b.account_key) return res.status(400).json({ error: 'account_key is required' });
@@ -423,7 +427,7 @@ router.post('/accounts', adminAuth, requirePermission('email.edit'), async (req,
// Test an inbound mailbox's IMAP connection (before or after saving). Resolves
// a masked/blank password from the stored row for the given account_key.
router.post('/accounts/test', adminAuth, requirePermission('email.view'), async (req, res) => {
router.post('/accounts/test', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const b = req.body || {};
const { isPrivateIP } = require('../utils/networkValidation');
@@ -710,7 +714,7 @@ router.get('/queue', adminAuth, requirePermission('email.view'), [
// 119); rows sent before that migration have none. Attachment disk paths in
// `email_data` are never exposed — only the filenames, so the pane can list
// attachments without leaking storage paths (same PII posture as the list).
router.get('/queue/:id', adminAuth, requirePermission('email.view'), async (req, res) => {
router.get('/queue/:id', adminAuth, messagingGate, requirePermission('email.view'), async (req, res) => {
try {
const id = parseInt(req.params.id, 10);
if (!Number.isInteger(id)) return res.status(400).json({ error: 'Invalid id' });
@@ -760,7 +764,7 @@ router.get('/queue/:id', adminAuth, requirePermission('email.view'), async (req,
// edited the body (reply or document message), so it is sent as-is — no
// template render — after a sanitize pass. Recorded in email_queue as a
// 'manual' send so it surfaces under Customers > Sent.
router.post('/send', adminAuth, requirePermission('email.send'), async (req, res) => {
router.post('/send', adminAuth, messagingGate, requirePermission('email.send'), async (req, res) => {
try {
const b = req.body || {};
const to = String(b.to || '').trim();
@@ -48,7 +48,15 @@ export const DocumentActionModal: React.FC<{
let cancelled = false;
setResolving(true);
customerAdminService.search(senderEmail)
.then((rows) => { if (!cancelled && rows.length) pick(rows[0]); })
.then((rows) => {
if (cancelled) return;
// search matches email/name/company PREFIXES — only auto-pick on an
// EXACT email match so a spoofed/partial sender can't prefill the wrong
// customer. Otherwise leave the picker for the admin to choose.
const target = senderEmail.trim().toLowerCase();
const exact = rows.find((r) => (r.email || '').toLowerCase() === target);
if (exact) pick(exact);
})
.catch(() => {})
.finally(() => { if (!cancelled) setResolving(false); });
return () => { cancelled = true; };
@@ -1,5 +1,6 @@
import React, { useEffect, useRef, useState } from 'react';
import { useMutation } from '@tanstack/react-query';
import DOMPurify from 'dompurify';
import { X, Send as SendIcon } from 'lucide-react';
import { toast } from 'react-toastify';
import { emailService } from '../../../services/email.service';
@@ -35,7 +36,9 @@ export const MessageComposer: React.FC<{
const bodyRef = useRef<HTMLDivElement>(null);
useEffect(() => {
if (bodyRef.current) bodyRef.current.innerHTML = init.html || '';
// Sanitize before it hits the contentEditable innerHTML — the initial body
// can include untrusted text (e.g. an inbound sender name in a reply stub).
if (bodyRef.current) bodyRef.current.innerHTML = DOMPurify.sanitize(init.html || '');
// Load initial body exactly once; further edits are the admin's.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, []);
@@ -55,6 +55,12 @@ const fmt = (s?: string | null) =>
// narrow sidebar); full address stays in the hover title.
const localPart = (addr?: string | null) => (addr ? `${addr.split('@')[0]}@` : '');
// Escape untrusted text before it goes into an HTML string. The inbound From
// header carries an attacker-controlled display name; the reply stub builds raw
// HTML for the (contentEditable) composer, so this MUST be escaped there.
const escapeHtml = (s: string) =>
s.replace(/[&<>"']/g, (c) => (({ '&': '&amp;', '<': '&lt;', '>': '&gt;', '"': '&quot;', "'": '&#39;' } as Record<string, string>)[c]));
const STATUS_STYLES: Record<string, string> = {
sent: 'bg-green-100 text-green-800 dark:bg-green-900/40 dark:text-green-300',
ingested: 'bg-green-100 text-green-800 dark:bg-green-900/40 dark:text-green-300',
@@ -527,7 +533,7 @@ const ReadingPane: React.FC<{
? () => {
const it = selection.item;
const subj = /^re:/i.test(it.subject || '') ? (it.subject || '') : `Re: ${it.subject || ''}`;
const quoted = `<p><br></p><p style="color:#888;font-size:12px">${t('messages.onWrote', 'On')} ${fmt(it.received_at)}, ${it.from_address}:</p>`;
const quoted = `<p><br></p><p style="color:#888;font-size:12px">${t('messages.onWrote', 'On')} ${fmt(it.received_at)}, ${escapeHtml(it.from_address || '')}:</p>`;
onCompose({ to: it.from_address || '', subject: subj, html: quoted, replyToReceivedId: it.id }, t('messages.reply', 'Reply'));
}
: undefined;