diff --git a/finbot/apps/admin/routes/api.py b/finbot/apps/admin/routes/api.py index bae0b174..45d7aaa4 100644 --- a/finbot/apps/admin/routes/api.py +++ b/finbot/apps/admin/routes/api.py @@ -335,6 +335,9 @@ async def send_message( bcc=req.bcc, ) + if result.get("error"): + raise HTTPException(status_code=400, detail=result["error"]) + external = [d for d in result.get("deliveries", []) if d["type"] == "external"] if external: from finbot.core.messaging import event_bus # pylint: disable=import-outside-toplevel diff --git a/finbot/apps/vendor/routes/api.py b/finbot/apps/vendor/routes/api.py index 555b727a..2005b698 100644 --- a/finbot/apps/vendor/routes/api.py +++ b/finbot/apps/vendor/routes/api.py @@ -1136,6 +1136,9 @@ async def send_message( bcc=req.bcc, ) + if result.get("error"): + raise HTTPException(status_code=400, detail=result["error"]) + external = [d for d in result.get("deliveries", []) if d["type"] == "external"] if external: await event_bus.emit_business_event( diff --git a/finbot/mcp/servers/finmail/routing.py b/finbot/mcp/servers/finmail/routing.py index 437f9f07..84e50351 100644 --- a/finbot/mcp/servers/finmail/routing.py +++ b/finbot/mcp/servers/finmail/routing.py @@ -17,6 +17,8 @@ logger = logging.getLogger(__name__) +MAX_EMAIL_ADDRESS_LENGTH = 254 + def get_admin_address(namespace: str) -> str: """Derive the canonical admin address from a namespace.""" @@ -48,6 +50,22 @@ def _is_internal_address(email_addr: str, namespace: str) -> bool: return email_addr.lower().endswith(f"@{namespace.lower()}.finbot") +def _normalize_and_validate_email_address(email_addr: str) -> tuple[str | None, str | None]: + """Normalize a recipient address and reject obviously invalid values.""" + normalized = email_addr.strip() if isinstance(email_addr, str) else "" + + if not normalized: + return None, "Email address is required" + + if len(normalized) > MAX_EMAIL_ADDRESS_LENGTH: + return ( + None, + f"Email address exceeds maximum length of {MAX_EMAIL_ADDRESS_LENGTH} characters", + ) + + return normalized, None + + def route_and_deliver( db: Session, repo: EmailRepository, @@ -75,6 +93,10 @@ def route_and_deliver( for role, addresses in [("to", to), ("cc", cc), ("bcc", bcc)]: for email_addr in (addresses or []): + email_addr, validation_error = _normalize_and_validate_email_address(email_addr) + if validation_error: + return {"error": validation_error} + visible_bcc = bcc_json if role == "bcc" else None vendor = ( diff --git a/finbot/static/js/admin/messages.js b/finbot/static/js/admin/messages.js index 20ff1fa9..9d6d3594 100644 --- a/finbot/static/js/admin/messages.js +++ b/finbot/static/js/admin/messages.js @@ -30,6 +30,8 @@ const TYPE_ICONS = { reminder: '', }; +const MAX_EMAIL_ADDRESS_LENGTH = 254; + ready(function () { initializeInbox(); }); @@ -377,8 +379,21 @@ function parseAddresses(value) { return value.split(',').map(s => s.trim()).filter(Boolean); } +function validateAddresses(addresses) { + if (!addresses) return null; + + const invalid = addresses.find(addr => addr.length > MAX_EMAIL_ADDRESS_LENGTH); + if (invalid) { + return `Each email address must be ${MAX_EMAIL_ADDRESS_LENGTH} characters or fewer`; + } + + return null; +} + async function sendComposedEmail() { const to = parseAddresses(document.getElementById('compose-to')?.value); + const cc = parseAddresses(document.getElementById('compose-cc')?.value); + const bcc = parseAddresses(document.getElementById('compose-bcc')?.value); const subject = document.getElementById('compose-subject')?.value?.trim(); const body = document.getElementById('compose-body')?.value?.trim(); @@ -386,13 +401,16 @@ async function sendComposedEmail() { if (!subject) return showNotification('Subject is required', 'error'); if (!body) return showNotification('Message body is required', 'error'); + const addressError = validateAddresses([...(to || []), ...(cc || []), ...(bcc || [])]); + if (addressError) return showNotification(addressError, 'error'); + const payload = { to, subject, body, message_type: 'general', - cc: parseAddresses(document.getElementById('compose-cc')?.value), - bcc: parseAddresses(document.getElementById('compose-bcc')?.value), + cc, + bcc, }; try { @@ -403,7 +421,8 @@ async function sendComposedEmail() { await loadMessages(); } catch (err) { console.error('Failed to send email:', err); - showNotification('Failed to send email', 'error'); + const message = err?.response?.data?.detail || 'Failed to send email'; + showNotification(message, 'error'); } } diff --git a/finbot/static/js/common/api.js b/finbot/static/js/common/api.js index d74a3289..7c5aad9d 100644 --- a/finbot/static/js/common/api.js +++ b/finbot/static/js/common/api.js @@ -73,8 +73,13 @@ class FinBotAPI { } if (!response.ok) { + const errorMessage = + data?.detail || + data?.message || + data?.error?.message || + `HTTP ${response.status}: ${response.statusText}`; throw new APIError( - data.message || `HTTP ${response.status}: ${response.statusText}`, + errorMessage, response.status, data ); @@ -284,7 +289,12 @@ function handleAPIError(error, options = {}) { } // Show user-friendly error message - const message = error.data?.message || error.message || 'An error occurred'; + const message = + error.data?.detail || + error.data?.message || + error.data?.error?.message || + error.message || + 'An error occurred'; if (options.showAlert !== false) { showNotification(message, 'danger'); diff --git a/finbot/static/js/vendor/messages.js b/finbot/static/js/vendor/messages.js index 5b63cbff..6f1cc07b 100644 --- a/finbot/static/js/vendor/messages.js +++ b/finbot/static/js/vendor/messages.js @@ -30,6 +30,8 @@ const TYPE_ICONS = { reminder: '', }; +const MAX_EMAIL_ADDRESS_LENGTH = 254; + ready(function () { initializeInbox(); }); @@ -385,8 +387,21 @@ function parseAddresses(value) { return value.split(',').map(s => s.trim()).filter(Boolean); } +function validateAddresses(addresses) { + if (!addresses) return null; + + const invalid = addresses.find(addr => addr.length > MAX_EMAIL_ADDRESS_LENGTH); + if (invalid) { + return `Each email address must be ${MAX_EMAIL_ADDRESS_LENGTH} characters or fewer`; + } + + return null; +} + async function sendComposedEmail() { const to = parseAddresses(document.getElementById('compose-to')?.value); + const cc = parseAddresses(document.getElementById('compose-cc')?.value); + const bcc = parseAddresses(document.getElementById('compose-bcc')?.value); const subject = document.getElementById('compose-subject')?.value?.trim(); const body = document.getElementById('compose-body')?.value?.trim(); @@ -394,13 +409,16 @@ async function sendComposedEmail() { if (!subject) return showNotification('Subject is required', 'error'); if (!body) return showNotification('Message body is required', 'error'); + const addressError = validateAddresses([...(to || []), ...(cc || []), ...(bcc || [])]); + if (addressError) return showNotification(addressError, 'error'); + const payload = { to, subject, body, message_type: 'general', - cc: parseAddresses(document.getElementById('compose-cc')?.value), - bcc: parseAddresses(document.getElementById('compose-bcc')?.value), + cc, + bcc, }; try { @@ -411,7 +429,8 @@ async function sendComposedEmail() { await loadMessages(); } catch (err) { console.error('Failed to send email:', err); - showNotification('Failed to send email', 'error'); + const message = err?.response?.data?.detail || 'Failed to send email'; + showNotification(message, 'error'); } }