Fix(finmail): reject malformed addresses before routing cascade (MCP-FM-ADDR-002)

[Jean-Regis-M]

Summary

Fixes #345

Malformed email addresses those missing @, starting with @, or ending with @ were silently
accepted by route_and_deliver, passed through the entire routing cascade without matching any known
inbox, and were stored as external dead-drop deliveries with sent: True returned to the caller.
This gave agents false confirmation of successful delivery for addresses that can never be valid.

---

Problem

route_and_deliver in finbot/mcp/servers/finmail/routing.py accepts a raw list of address strings
and immediately enters the vendor → admin → internal → user lookup cascade with no prior format check.

A string like "no-at-sign" will:

1. Fail the vendor DB lookup (no matching Vendor.email)
2. Fail the admin address comparison (get_admin_address returns admin@<ns>.finbot)
3. Fail the internal domain check (_is_internal_address looks for @<ns>.finbot suffix)
4. Fail the user DB lookup (no matching User.email)
5. Fall through to the external dead-drop stored with inbox_type="external", logged as a
   warning, and counted as a successful send

The function then returns:

{
  "sent": true,
  "subject": "...",
  "deliveries": [{ "type": "external", "email": "no-at-sign", "role": "to" }],
  "delivery_count": 0
}

The agent receives sent: true with no error, no rejection signal, and no indication the address
was invalid. The malformed address is persisted to the database.

---

Root Cause

route_and_deliver()
  └─ for email_addr in addresses:          ← no validation here
       ├─ vendor lookup       → miss
       ├─ admin check         → miss
       ├─ internal check      → miss
       ├─ user lookup         → miss
       └─ external dead-drop  ← malformed address lands here, stored, sent=True returned

There is no RFC 5322 / structural format guard anywhere in the routing path. Any non-empty string
is a valid input to the cascade.

---

Solution

Insert a three-condition @ guard as the first statement inside the per-address loop, before
any DB query is attempted:

# Before (no validation)
for email_addr in (addresses or []):
    visible_bcc = bcc_json if role == "bcc" else None
    vendor = db.query(Vendor)...

# After
for email_addr in (addresses or []):
    if "@" not in email_addr or email_addr.startswith("@") or email_addr.endswith("@"):
        deliveries.append({
            "type": "undeliverable",
            "email": email_addr,
            "role": role,
            "reason": "invalid_format",
        })
        continue

    visible_bcc = bcc_json if role == "bcc" else None
    vendor = db.query(Vendor)...

Malformed addresses are recorded as type: undeliverable and skipped via continue.
No other code paths are modified.

Why undeliverable and not an exception?
The existing return schema already excludes "undeliverable" from delivery_count
(d["type"] not in ("undeliverable", "external")). Using the same pattern keeps the
return contract stable and allows callers to inspect which addresses failed without
crashing a multi-recipient send.

---

What I changed

File	Change
finbot/mcp/servers/finmail/routing.py	+4 lines format guard at top of per-address loop

Diff footprint: 4 lines added, 0 deleted, 1 file touched.

---

Edge Cases Covered

Input	Condition triggered	Result
"no-at-sign"	"@" not in email_addr	undeliverable
"@domain.com"	startswith("@")	undeliverable
"user@"	endswith("@")	undeliverable
"" (empty string)	"@" not in ""	undeliverable
"user@domain.com"	None guard passes	Existing routing unchanged
"user@@domain.com"	None contains @, no leading/trailing	Existing routing (out of scope)

---

Impact

• No breaking changes return shape is unchanged
• delivery_count already excludes undeliverable no aggregation fix needed
• All valid-address routing paths are completely untouched
• No new imports, no dependency changes, no API contract changes
• Callers can detect rejection by checking deliveries[n].type == "undeliverable"

---

Testing

Failing test this fixes:

pytest tests/unit/mcp/test_finmail.py::TestEmailAddressValidation::test_fm_addr_002_email_address_without_at_symbol_accepted -v

Regression guard:

pytest tests/unit/mcp/test_finmail.py::TestEmailSend::test_fm_send_001_send_email_to_vendor_inbox -v

Manual smoke check:

# Should return undeliverable, not external
result = route_and_deliver(db, repo, "acme", to=["no-at-sign"], subject="test", body="test")
assert result["deliveries"][0]["type"] == "undeliverable"
assert result["deliveries"][0]["reason"] == "invalid_format"
assert result["delivery_count"] == 0