cleanup pass — 17 findings from Opus code-quality audit
Applied from the cleanup-agent report (separate from the security
audit's 18 fixes earlier today):
HIGH:
- HIGH-1: replaced hand-rolled civil_from_unix + chrono_rfc3339_now
with chrono::Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true).
~50 LOC of brittle Hinnant-algorithm civil-calendar math gone.
Plus 5 unit tests retired with the function. chrono pulls a small
default-features=false slice (clock + serde-not-included).
- HIGH-2: extracted shared reject_imap_unsafe() helper. validate_mailbox
and the mail_thread message_id check both go through it. The
message_id check now also rejects '{' (literal-form opener) for
symmetry — same byte set as validate_mailbox.
- HIGH-3: mail_reply uses smtp::ensure_angle_brackets() on the parent
Message-Id + each References entry. mail-parser strips brackets;
lettre writes through verbatim; strict RFC-5322 receivers will drop
the threading link if brackets are missing. Now canonical.
- HIGH-4: extract_addr moved from tools.rs to smtp.rs as
smtp::extract_bare_addr. Module hygiene — RFC-5322 mailbox parsing
belongs in the SMTP-side module, not the rmcp surface.
- HIGH-5: mail_reply Re:-prefix check now non-allocating —
subject.get(..3).map(|s| s.eq_ignore_ascii_case("re:")) instead of
.to_ascii_lowercase().starts_with("re:") which allocated a fresh
String for the comparison.
MED:
- MED-1: dropped thiserror dep (workspace + crate). Never derived.
- MED-6: ReadOutput.headers is now typed BTreeMap<String,String> instead
of serde_json::Value::Object. Wire JSON shape unchanged; downstream
consumers can .get(name) directly without the .as_str() dance.
- MED-8: fetch_to_list_entry returns Option<ListEntry> and drops
entries when the server omits UID. Was uid=0 silent fallback;
now we log a warning and skip.
- MED-10: introduced PRIMARY_BODY_PART = 0 const, replaced 4 magic 0s
at parsed.body_text(0) / parsed.body_html(0) call sites.
- MED-11: skip insert of empty-valued headers in the flat headers map.
was producing "key":"" entries for headers mail-parser couldn't
render to a flat string.
LOW:
- LOW-1: collapsed MailService { inner: Arc<MailInner> } to
MailService { config: Arc<Config> }. The MailInner wrapper served no
purpose with a single field.
- LOW-2: rewrote to_field_collapses_to_vec test with let bindings
instead of single-arm match.
- LOW-4: format_imap_since year range tightened from 1900..=9999 to
1970..=9999 (unix-epoch floor; we don't use pre-epoch IMAP SINCE).
- LOW-5: promoted max_encoded to MAX_ATTACHMENT_BASE64_BYTES const.
- LOW-6: SendOutput now #[derive(Serialize)] — mail_send and
mail_reply tools use it via the new IntoMcpError trait instead of
serde_json::json!() boilerplate.
- LOW-7: added IntoMcpError trait — anyhow::Result<T: Serialize>
-> Result<String, String>. Removes 10 copy-pasted
.map_err(|e| format!("{e:#}"))? + serialize chains.
- LOW-9: documented the 20 MB read cap vs 25 MB send cap asymmetry
via comments on both consts.
- LOW-10: UID MOVE fallback log demoted to trace! and renamed field
imap_mv_err so log analytics doesn't flag the graceful fallback as
an error.
- LOW-13: SMTP From header built via Mailbox::new() instead of
format!("{} <{}>")-then-.parse(). One alloc, one parse pass gone.
INFO:
- INFO-3: lettre 'hostname' feature dropped from Cargo.toml. We
override Message-ID with our own UUID@from_domain; lettre never
needed the system hostname.
Deferred from this pass:
- MED-2 with_session wrapper — substantial refactor across 8 IMAP
functions for moderate DRY win; saving for a Phase E lifecycle pass.
- MED-7 / LOW-14 typed address shape — would change wire JSON for
mail_inbox_list/read; backwards-incompatible.
- MED-12 narrow UID MOVE fallback — needs async-imap error-variant
taxonomy research.
- LOW-11 / LOW-12 stringly format / action enums — auditor flagged as
marginal; keep stringly with description-enumerated values.
Test count: 33 -> 31 (-5 civil_from_unix, +3 extract_bare_addr, +3
ensure_angle_brackets, -1 stale extract_addr in wrong module). All
passing. Wire smoke verified — send / list / read round trip clean,
headers map is now a flat dict with no empties, chrono-rendered
timestamps match the prior shape.
This commit is contained in:
parent
b681953824
commit
7c8e246544
6 changed files with 244 additions and 243 deletions
14
Cargo.lock
generated
14
Cargo.lock
generated
|
|
@ -567,17 +567,6 @@ version = "0.5.0"
|
|||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "2304e00983f87ffb38b55b444b5e3b60a884b5d30c0fca7d82fe33449bbe55ea"
|
||||
|
||||
[[package]]
|
||||
name = "hostname"
|
||||
version = "0.4.2"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "617aaa3557aef3810a6369d0a99fac8a080891b68bd9f9812a1eeda0c0730cbd"
|
||||
dependencies = [
|
||||
"cfg-if",
|
||||
"libc",
|
||||
"windows-link",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "httpdate"
|
||||
version = "1.0.3"
|
||||
|
|
@ -791,7 +780,6 @@ dependencies = [
|
|||
"fastrand",
|
||||
"futures-io",
|
||||
"futures-util",
|
||||
"hostname",
|
||||
"httpdate",
|
||||
"idna",
|
||||
"mime",
|
||||
|
|
@ -849,6 +837,7 @@ dependencies = [
|
|||
"anyhow",
|
||||
"async-imap",
|
||||
"base64 0.22.1",
|
||||
"chrono",
|
||||
"dirs 5.0.1",
|
||||
"futures",
|
||||
"lettre",
|
||||
|
|
@ -860,7 +849,6 @@ dependencies = [
|
|||
"serde",
|
||||
"serde_json",
|
||||
"shellexpand",
|
||||
"thiserror 1.0.69",
|
||||
"tokio",
|
||||
"tokio-rustls",
|
||||
"toml",
|
||||
|
|
|
|||
14
Cargo.toml
14
Cargo.toml
|
|
@ -25,12 +25,13 @@ schemars = "0.8"
|
|||
|
||||
# SMTP — lettre handles RFC-5322 headers (Date, Message-ID), STARTTLS,
|
||||
# multipart/alternative + multipart/mixed natively. rustls-tls so we
|
||||
# don't pull openssl.
|
||||
# don't pull openssl. No `hostname` feature — we override Message-ID
|
||||
# with our own UUID@<from_domain>, so lettre never needs the system
|
||||
# hostname.
|
||||
lettre = { version = "0.11", default-features = false, features = [
|
||||
"tokio1-rustls-tls",
|
||||
"smtp-transport",
|
||||
"builder",
|
||||
"hostname",
|
||||
] }
|
||||
|
||||
# IMAP — async-imap is tokio-native and supports UID-based addressing
|
||||
|
|
@ -58,13 +59,18 @@ uuid = { version = "1", features = ["v4"] }
|
|||
# Base64 for attachments
|
||||
base64 = "0.22"
|
||||
|
||||
# Errors
|
||||
# Errors — anyhow at module boundaries; rmcp tool methods return
|
||||
# `Result<String, String>` and convert via the IntoMcpError trait.
|
||||
anyhow = "1"
|
||||
thiserror = "1"
|
||||
|
||||
# Stream adapter (.next() on async-imap fetch streams)
|
||||
futures = "0.3"
|
||||
|
||||
# RFC-3339 timestamps for SendOutput.sent_at and parsed header dates.
|
||||
# default-features=false keeps us off the system-locale crate; we only
|
||||
# need the UTC clock + serialization helpers.
|
||||
chrono = { version = "0.4", default-features = false, features = ["clock"] }
|
||||
|
||||
# Logging — stderr only, never stdout (stdio is the MCP transport).
|
||||
tracing = "0.1"
|
||||
tracing-subscriber = { version = "0.3", features = ["env-filter"] }
|
||||
|
|
|
|||
|
|
@ -34,8 +34,8 @@ uuid = { workspace = true }
|
|||
base64 = { workspace = true }
|
||||
|
||||
anyhow = { workspace = true }
|
||||
thiserror = { workspace = true }
|
||||
futures = { workspace = true }
|
||||
chrono = { workspace = true }
|
||||
tracing = { workspace = true }
|
||||
tracing-subscriber = { workspace = true }
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
//! UID-based addressing throughout (UID stays stable across folder selects,
|
||||
//! sequence numbers don't).
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::{anyhow, Context, Result};
|
||||
|
|
@ -21,6 +22,12 @@ use tokio_rustls::TlsConnector;
|
|||
|
||||
use crate::config::Account;
|
||||
|
||||
/// mail-parser's body-part index for the primary text body. The 0 means
|
||||
/// "first text/* leaf in the structure"; the full picker docs live in the
|
||||
/// mail-parser crate. Pull it into a named constant so the read paths
|
||||
/// aren't peppered with bare `0` literals.
|
||||
const PRIMARY_BODY_PART: usize = 0;
|
||||
|
||||
#[derive(Debug, Clone, Default)]
|
||||
pub struct ListOpts {
|
||||
pub since: Option<String>, // YYYY-MM-DD
|
||||
|
|
@ -65,7 +72,12 @@ pub struct ReadOutput {
|
|||
pub cc: Vec<String>,
|
||||
pub subject: String,
|
||||
pub date: Option<String>,
|
||||
pub headers: serde_json::Value,
|
||||
/// Flat header name → raw value map. Typed `BTreeMap<String, String>`
|
||||
/// rather than `serde_json::Value` so consumers can call `.get(name)`
|
||||
/// without the `as_str()` dance. Empty-valued headers are omitted
|
||||
/// (mail-parser sometimes emits structured headers we can't decode
|
||||
/// back to a flat string — we don't want those landing as `""`).
|
||||
pub headers: BTreeMap<String, String>,
|
||||
pub body: String,
|
||||
pub format: String,
|
||||
pub attachments: Vec<AttachmentMeta>,
|
||||
|
|
@ -89,9 +101,12 @@ pub struct FolderEntry {
|
|||
|
||||
const DEFAULT_LIMIT: u32 = 50;
|
||||
const MAX_LIMIT: u32 = 500;
|
||||
/// Cap on raw_eml fetch size in `mail_inbox_read`. Anything larger refuses
|
||||
/// the read with a hint to use `format=text` (which still pulls the body,
|
||||
/// but doesn't double-buffer through `String::from_utf8_lossy`).
|
||||
|
||||
/// Cap on raw_eml fetch size in `mail_inbox_read`. Asymmetric with
|
||||
/// `smtp::MAX_ATTACHMENT_BYTES` (25 MB) by design — we can RECEIVE more
|
||||
/// than we'll let the LLM SEND because rspamd / postfix-relay can rewrite
|
||||
/// + inject headers on inbound mail. On the read side we want to refuse
|
||||
/// very large messages before mail-parser allocates them into RAM.
|
||||
const MAX_RAW_EML_BYTES: u64 = 20 * 1024 * 1024;
|
||||
|
||||
/// Clamp a caller-provided limit to `[1, MAX_LIMIT]` with `DEFAULT_LIMIT`
|
||||
|
|
@ -105,23 +120,27 @@ fn clamp_limit(limit: Option<u32>) -> u32 {
|
|||
}
|
||||
}
|
||||
|
||||
/// Mailbox-name guard: reject any byte that would force us into IMAP
|
||||
/// quoted-string or literal-form territory we don't control. CR/LF/NUL
|
||||
/// could split commands; `\`/`"` would need escaping that async-imap's
|
||||
/// `uid_copy` does not perform on the destination argument. We refuse
|
||||
/// the small set rather than try to quote-and-escape on the wire.
|
||||
fn validate_mailbox(name: &str) -> Result<()> {
|
||||
if name.is_empty() {
|
||||
return Err(anyhow!("mailbox name cannot be empty"));
|
||||
/// Reject any byte that would force us into IMAP quoted-string or literal-
|
||||
/// form territory we don't control. CR/LF/NUL could split commands;
|
||||
/// `\`/`"` would need escaping that async-imap doesn't perform consistently
|
||||
/// on every command argument (it quotes select/list but not uid_copy
|
||||
/// destinations). `{` opens IMAP literal-form. We refuse the small set
|
||||
/// rather than try to quote-and-escape per-call.
|
||||
///
|
||||
/// Shared by `validate_mailbox` and the `mail_thread` Message-ID check.
|
||||
/// `label` is what the field is called in the error message.
|
||||
fn reject_imap_unsafe(value: &str, label: &str) -> Result<()> {
|
||||
if value.is_empty() {
|
||||
return Err(anyhow!("{label} cannot be empty"));
|
||||
}
|
||||
for b in name.bytes() {
|
||||
for b in value.bytes() {
|
||||
match b {
|
||||
b'\r' | b'\n' | 0 => {
|
||||
return Err(anyhow!("mailbox name contains CR/LF/NUL"));
|
||||
return Err(anyhow!("{label} contains CR/LF/NUL"));
|
||||
}
|
||||
b'"' | b'\\' => {
|
||||
b'"' | b'\\' | b'{' => {
|
||||
return Err(anyhow!(
|
||||
"mailbox name contains `\"` or `\\` — refused; rename the folder server-side if needed"
|
||||
"{label} contains `\"`, `\\`, or `{{` — refused"
|
||||
));
|
||||
}
|
||||
_ => {}
|
||||
|
|
@ -130,6 +149,13 @@ fn validate_mailbox(name: &str) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Mailbox-name guard. Thin wrapper over `reject_imap_unsafe` so callers
|
||||
/// read at the call site as `validate_mailbox(folder)?` rather than
|
||||
/// repeating the label string everywhere.
|
||||
fn validate_mailbox(name: &str) -> Result<()> {
|
||||
reject_imap_unsafe(name, "mailbox name")
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
// list
|
||||
// =============================================================================
|
||||
|
|
@ -188,9 +214,10 @@ pub async fn list(account: &Account, opts: ListOpts) -> Result<Vec<ListEntry>> {
|
|||
|
||||
while let Some(msg_res) = stream.next().await {
|
||||
let msg = msg_res.context("UID FETCH stream item")?;
|
||||
let entry = fetch_to_list_entry(&msg);
|
||||
if let Some(entry) = fetch_to_list_entry(&msg) {
|
||||
out.push(entry);
|
||||
}
|
||||
}
|
||||
drop(stream);
|
||||
|
||||
session.logout().await.ok();
|
||||
|
|
@ -199,8 +226,18 @@ pub async fn list(account: &Account, opts: ListOpts) -> Result<Vec<ListEntry>> {
|
|||
Ok(out)
|
||||
}
|
||||
|
||||
fn fetch_to_list_entry(msg: &Fetch) -> ListEntry {
|
||||
let uid = msg.uid.unwrap_or(0);
|
||||
fn fetch_to_list_entry(msg: &Fetch) -> Option<ListEntry> {
|
||||
// Server SHOULD return UID when we asked for it in the fetch spec; if
|
||||
// it didn't, drop the entry rather than emit `uid=0` (which would
|
||||
// collide with any other broken entry and resolve to "no such message"
|
||||
// on a follow-up read).
|
||||
let uid = match msg.uid {
|
||||
Some(u) => u,
|
||||
None => {
|
||||
tracing::warn!("FETCH response missing UID — dropping entry");
|
||||
return None;
|
||||
}
|
||||
};
|
||||
let flags: Vec<String> = msg.flags().map(|f| render_flag(&f)).collect();
|
||||
let header_bytes = msg.header().unwrap_or(&[]);
|
||||
|
||||
|
|
@ -232,7 +269,7 @@ fn fetch_to_list_entry(msg: &Fetch) -> ListEntry {
|
|||
})
|
||||
.unwrap_or(false);
|
||||
|
||||
ListEntry {
|
||||
Some(ListEntry {
|
||||
uid,
|
||||
message_id,
|
||||
from,
|
||||
|
|
@ -241,7 +278,7 @@ fn fetch_to_list_entry(msg: &Fetch) -> ListEntry {
|
|||
date,
|
||||
has_attachments,
|
||||
flags,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
|
@ -320,14 +357,14 @@ pub async fn read(
|
|||
let body = match format {
|
||||
"raw_eml" => String::from_utf8_lossy(&raw_body).into_owned(),
|
||||
"html" => parsed
|
||||
.body_html(0)
|
||||
.body_html(PRIMARY_BODY_PART)
|
||||
.map(|s| s.into_owned())
|
||||
.or_else(|| parsed.body_text(0).map(|s| s.into_owned()))
|
||||
.or_else(|| parsed.body_text(PRIMARY_BODY_PART).map(|s| s.into_owned()))
|
||||
.unwrap_or_default(),
|
||||
_ => parsed
|
||||
.body_text(0)
|
||||
.body_text(PRIMARY_BODY_PART)
|
||||
.map(|s| s.into_owned())
|
||||
.or_else(|| parsed.body_html(0).map(|s| s.into_owned()))
|
||||
.or_else(|| parsed.body_html(PRIMARY_BODY_PART).map(|s| s.into_owned()))
|
||||
.unwrap_or_default(),
|
||||
};
|
||||
|
||||
|
|
@ -349,23 +386,24 @@ pub async fn read(
|
|||
})
|
||||
.collect();
|
||||
|
||||
// Headers as a flat JSON map. mail-parser's `HeaderValue::as_text()`
|
||||
// returns None for structured variants (Address / DateTime / ContentType
|
||||
// / Received) which would leave most "interesting" headers empty. We use
|
||||
// `Message::header_raw(name)` instead — that returns the un-decoded
|
||||
// header value as &str, uniform across all header types.
|
||||
let mut headers = serde_json::Map::new();
|
||||
let mut seen = std::collections::HashSet::new();
|
||||
// Headers as a flat name→raw-value map. mail-parser's `HeaderValue::
|
||||
// as_text()` returns None for structured variants (Address / DateTime /
|
||||
// ContentType / Received), so we use `Message::header_raw(name)` to
|
||||
// get the un-decoded header value as &str — uniform across all
|
||||
// header types. Empty-valued headers are skipped (would surface as
|
||||
// `""` entries the caller has to filter anyway).
|
||||
let mut headers: BTreeMap<String, String> = BTreeMap::new();
|
||||
for h in parsed.headers() {
|
||||
let name = h.name().to_string();
|
||||
if !seen.insert(name.clone()) {
|
||||
let name = h.name();
|
||||
if headers.contains_key(name) {
|
||||
continue; // duplicate header — keep the first occurrence
|
||||
}
|
||||
let raw = parsed
|
||||
.header_raw(name.as_str())
|
||||
.map(|s| s.trim().to_string())
|
||||
.unwrap_or_default();
|
||||
headers.insert(name, serde_json::Value::String(raw));
|
||||
if let Some(raw) = parsed.header_raw(name) {
|
||||
let trimmed = raw.trim();
|
||||
if !trimmed.is_empty() {
|
||||
headers.insert(name.to_string(), trimmed.to_string());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let subject = parsed.subject().unwrap_or_default().to_string();
|
||||
|
|
@ -378,7 +416,7 @@ pub async fn read(
|
|||
cc: addr_list(parsed.cc()),
|
||||
subject,
|
||||
date: parsed.date().map(|d| d.to_rfc3339()),
|
||||
headers: serde_json::Value::Object(headers),
|
||||
headers,
|
||||
body,
|
||||
format: format.to_string(),
|
||||
attachments,
|
||||
|
|
@ -487,16 +525,7 @@ pub async fn thread(
|
|||
if id_unbraced.is_empty() {
|
||||
return Err(anyhow!("message_id is empty"));
|
||||
}
|
||||
// Reject `"` (IMAP quoted-string terminator), `\` (IMAP quoted-string
|
||||
// escape), CR/LF (command terminators), and `{` (literal-form opener).
|
||||
if id_unbraced
|
||||
.chars()
|
||||
.any(|c| matches!(c, '"' | '\\' | '\r' | '\n' | '{'))
|
||||
{
|
||||
return Err(anyhow!(
|
||||
r#"message_id must not contain ", \, CR, LF, or {{"#
|
||||
));
|
||||
}
|
||||
reject_imap_unsafe(id_unbraced, "message_id")?;
|
||||
let folder = folder.unwrap_or("INBOX");
|
||||
validate_mailbox(folder)?;
|
||||
let limit = clamp_limit(limit);
|
||||
|
|
@ -814,7 +843,9 @@ async fn fetch_summaries(
|
|||
let mut out: Vec<ListEntry> = Vec::with_capacity(uids.len());
|
||||
while let Some(msg_res) = stream.next().await {
|
||||
let msg = msg_res.context("UID FETCH stream item")?;
|
||||
out.push(fetch_to_list_entry(&msg));
|
||||
if let Some(entry) = fetch_to_list_entry(&msg) {
|
||||
out.push(entry);
|
||||
}
|
||||
}
|
||||
drop(stream);
|
||||
// Default to newest-first; callers (thread) re-sort if they want oldest-first.
|
||||
|
|
@ -853,8 +884,8 @@ fn format_imap_since(iso_date: &str) -> Result<String> {
|
|||
let y: u32 = parts[0].parse().context("year")?;
|
||||
let m: u32 = parts[1].parse().context("month")?;
|
||||
let d: u32 = parts[2].parse().context("day")?;
|
||||
if !(1900..=9999).contains(&y) {
|
||||
return Err(anyhow!("year out of range (1900..=9999)"));
|
||||
if !(1970..=9999).contains(&y) {
|
||||
return Err(anyhow!("year out of range (1970..=9999)"));
|
||||
}
|
||||
if !(1..=31).contains(&d) {
|
||||
return Err(anyhow!("day out of range (1..=31)"));
|
||||
|
|
|
|||
|
|
@ -17,10 +17,12 @@
|
|||
|
||||
use anyhow::{anyhow, Context, Result};
|
||||
use base64::Engine;
|
||||
use chrono::{SecondsFormat, Utc};
|
||||
use lettre::message::header::ContentType;
|
||||
use lettre::message::{Attachment, MultiPart, SinglePart};
|
||||
use lettre::message::{Attachment, Mailbox, MultiPart, SinglePart};
|
||||
use lettre::transport::smtp::authentication::Credentials;
|
||||
use lettre::{AsyncSmtpTransport, AsyncTransport, Message, Tokio1Executor};
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::config::Account;
|
||||
|
||||
|
|
@ -44,7 +46,7 @@ pub struct SendInput {
|
|||
pub references: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
#[derive(Debug, Clone, Serialize)]
|
||||
pub struct SendOutput {
|
||||
pub message_id: String,
|
||||
pub sent_at: String, // RFC-3339
|
||||
|
|
@ -56,7 +58,13 @@ const USER_AGENT: &str = concat!("mail-mcp/", env!("CARGO_PKG_VERSION"));
|
|||
/// Match Gmail's effective 25 MB per-message ceiling — any single attachment
|
||||
/// past this is almost certainly unintended. Body caps are generous; HTML
|
||||
/// bodies past 5 MB are a smell.
|
||||
///
|
||||
/// Note the asymmetry with imap.rs's MAX_RAW_EML_BYTES (20 MB) — we can
|
||||
/// RECEIVE more than we'll let the LLM SEND because rspamd/postfix-relay
|
||||
/// can rewrite + add headers; on the read side we want to refuse very
|
||||
/// large messages before parsing them into RAM.
|
||||
const MAX_ATTACHMENT_BYTES: usize = 25 * 1024 * 1024;
|
||||
const MAX_ATTACHMENT_BASE64_BYTES: usize = (MAX_ATTACHMENT_BYTES * 4 + 2) / 3;
|
||||
const MAX_ATTACHMENTS: usize = 25;
|
||||
const MAX_BODY_BYTES: usize = 5 * 1024 * 1024;
|
||||
const MAX_TOTAL_RECIPIENTS: usize = 100;
|
||||
|
|
@ -97,9 +105,8 @@ pub fn validate_send_input(input: &SendInput) -> Result<()> {
|
|||
// Coarse pre-decode size estimate — base64 expands the payload ~33%,
|
||||
// so the encoded string is ~4*N/3 of the decoded bytes. Refuse early
|
||||
// if the encoded form alone exceeds 4/3 of MAX, saving a giant decode.
|
||||
let max_encoded = (MAX_ATTACHMENT_BYTES * 4 + 2) / 3;
|
||||
for att in &input.attachments {
|
||||
if att.content_base64.len() > max_encoded {
|
||||
if att.content_base64.len() > MAX_ATTACHMENT_BASE64_BYTES {
|
||||
return Err(anyhow!(
|
||||
"attachment `{}` encoded length {} exceeds limit (decoded cap {MAX_ATTACHMENT_BYTES})",
|
||||
att.filename,
|
||||
|
|
@ -110,13 +117,48 @@ pub fn validate_send_input(input: &SendInput) -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
/// Strip an RFC-5322 mailbox down to the bare address, dropping any display
|
||||
/// name. `Kayos <kayos@sulkta.com>` -> `kayos@sulkta.com`; `kayos@sulkta.com`
|
||||
/// passes through unchanged. Used by `mail_reply` because lettre's mailbox
|
||||
/// parser is strict — we feed it bare addresses extracted from the original
|
||||
/// `From` line rather than re-rendering names.
|
||||
pub fn extract_bare_addr(s: &str) -> String {
|
||||
if let (Some(lt), Some(gt)) = (s.find('<'), s.rfind('>')) {
|
||||
if lt < gt {
|
||||
return s[lt + 1..gt].trim().to_string();
|
||||
}
|
||||
}
|
||||
s.trim().to_string()
|
||||
}
|
||||
|
||||
/// Ensure a Message-ID-shaped value carries the canonical `<id@host>` form.
|
||||
/// `mail-parser` strips the brackets when it returns Message-Id / References
|
||||
/// values; lettre's `in_reply_to()` / `references()` write the bytes through
|
||||
/// verbatim, and strict RFC-5322 receivers will drop the threading link if
|
||||
/// the brackets are missing.
|
||||
pub fn ensure_angle_brackets(s: &str) -> String {
|
||||
let trimmed = s.trim();
|
||||
if trimmed.starts_with('<') && trimmed.ends_with('>') {
|
||||
trimmed.to_string()
|
||||
} else {
|
||||
format!("<{trimmed}>")
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn send(account: &Account, input: SendInput) -> Result<SendOutput> {
|
||||
validate_send_input(&input)?;
|
||||
|
||||
// Build From, To, Cc, Bcc.
|
||||
let from_str = format!("{} <{}>", account.from_name, account.from_addr);
|
||||
// Use Mailbox::new directly instead of round-tripping through a string —
|
||||
// if `from_name` ever contains `<` or `>`, the formatted-then-parsed path
|
||||
// breaks unhelpfully.
|
||||
let from_addr = account
|
||||
.from_addr
|
||||
.parse()
|
||||
.with_context(|| format!("parse from address `{}`", account.from_addr))?;
|
||||
let from_mb = Mailbox::new(Some(account.from_name.clone()), from_addr);
|
||||
let mut builder = Message::builder()
|
||||
.from(from_str.parse().context("parse from address")?)
|
||||
.from(from_mb)
|
||||
.subject(&input.subject);
|
||||
|
||||
for addr in &input.to {
|
||||
|
|
@ -174,7 +216,7 @@ pub async fn send(account: &Account, input: SendInput) -> Result<SendOutput> {
|
|||
.build()
|
||||
};
|
||||
|
||||
let sent_at = chrono_rfc3339_now();
|
||||
let sent_at = Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true);
|
||||
transport
|
||||
.send(email)
|
||||
.await
|
||||
|
|
@ -249,46 +291,6 @@ fn build_body(input: &SendInput) -> Result<MultiPart> {
|
|||
Ok(mixed)
|
||||
}
|
||||
|
||||
/// Render `now` as RFC-3339 UTC (`2026-05-21T06:42:18Z`). We avoid pulling
|
||||
/// the full `chrono` crate just for this; build the string by hand from
|
||||
/// std time.
|
||||
fn chrono_rfc3339_now() -> String {
|
||||
use std::time::{SystemTime, UNIX_EPOCH};
|
||||
let secs = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.map(|d| d.as_secs())
|
||||
.unwrap_or(0);
|
||||
// RFC-3339 via the same algorithm `httpdate` uses, simplified for UTC.
|
||||
let (year, month, day, hour, minute, second) = civil_from_unix(secs as i64);
|
||||
format!(
|
||||
"{:04}-{:02}-{:02}T{:02}:{:02}:{:02}Z",
|
||||
year, month, day, hour, minute, second
|
||||
)
|
||||
}
|
||||
|
||||
/// Convert a unix timestamp (UTC) to civil (Y,M,D,h,m,s). Copy of the
|
||||
/// classic Howard Hinnant algorithm — works for the full proleptic Gregorian range.
|
||||
fn civil_from_unix(t: i64) -> (i64, u32, u32, u32, u32, u32) {
|
||||
let days = t.div_euclid(86_400);
|
||||
let secs_of_day = t.rem_euclid(86_400);
|
||||
let hour = (secs_of_day / 3600) as u32;
|
||||
let minute = ((secs_of_day % 3600) / 60) as u32;
|
||||
let second = (secs_of_day % 60) as u32;
|
||||
|
||||
// 0000-03-01 is the start of the cycle ("era").
|
||||
let z = days + 719_468;
|
||||
let era = if z >= 0 { z } else { z - 146_096 } / 146_097;
|
||||
let doe = (z - era * 146_097) as u64;
|
||||
let yoe =
|
||||
(doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365;
|
||||
let y = yoe as i64 + era * 400;
|
||||
let doy = doe - (365 * yoe + yoe / 4 - yoe / 100);
|
||||
let mp = (5 * doy + 2) / 153;
|
||||
let d = (doy - (153 * mp + 2) / 5 + 1) as u32;
|
||||
let m = if mp < 10 { (mp + 3) as u32 } else { (mp - 9) as u32 };
|
||||
let y = if m <= 2 { y + 1 } else { y };
|
||||
(y, m, d, hour, minute, second)
|
||||
}
|
||||
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
@ -402,39 +404,54 @@ mod tests {
|
|||
assert!(validate_send_input(&i).is_ok());
|
||||
}
|
||||
|
||||
// ----- civil_from_unix -----
|
||||
// ----- extract_bare_addr -----
|
||||
|
||||
#[test]
|
||||
fn civil_from_unix_epoch() {
|
||||
assert_eq!(civil_from_unix(0), (1970, 1, 1, 0, 0, 0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn civil_from_unix_y2k() {
|
||||
// 2000-01-01 00:00:00 UTC = 946684800
|
||||
assert_eq!(civil_from_unix(946_684_800), (2000, 1, 1, 0, 0, 0));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn civil_from_unix_2026_05_21() {
|
||||
// 2026-05-21 13:44:03 UTC = 1779371043
|
||||
fn extract_bare_addr_strips_display_name() {
|
||||
assert_eq!(
|
||||
civil_from_unix(1_779_371_043),
|
||||
(2026, 5, 21, 13, 44, 3)
|
||||
extract_bare_addr("Kayos <kayos@sulkta.com>"),
|
||||
"kayos@sulkta.com"
|
||||
);
|
||||
assert_eq!(
|
||||
extract_bare_addr("\"Cobb Hayes\" <cobb@sulkta.com>"),
|
||||
"cobb@sulkta.com"
|
||||
);
|
||||
assert_eq!(extract_bare_addr(" spaces <a@b.com> "), "a@b.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn civil_from_unix_pre_epoch() {
|
||||
// 1969-12-31 23:59:59 UTC = -1
|
||||
assert_eq!(civil_from_unix(-1), (1969, 12, 31, 23, 59, 59));
|
||||
// 1969-01-01 00:00:00 UTC = -31536000
|
||||
assert_eq!(civil_from_unix(-31_536_000), (1969, 1, 1, 0, 0, 0));
|
||||
fn extract_bare_addr_passes_bare_address() {
|
||||
assert_eq!(extract_bare_addr("plain@addr.com"), "plain@addr.com");
|
||||
assert_eq!(extract_bare_addr(" trim-me@x.com "), "trim-me@x.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn civil_from_unix_leap_year() {
|
||||
// 2024-02-29 00:00:00 UTC = 1709164800 (leap day)
|
||||
assert_eq!(civil_from_unix(1_709_164_800), (2024, 2, 29, 0, 0, 0));
|
||||
fn extract_bare_addr_handles_garbage_gracefully() {
|
||||
assert_eq!(extract_bare_addr("> a@b.com <"), "> a@b.com <");
|
||||
assert_eq!(extract_bare_addr(""), "");
|
||||
assert_eq!(extract_bare_addr("Just A Name"), "Just A Name");
|
||||
}
|
||||
|
||||
// ----- ensure_angle_brackets -----
|
||||
|
||||
#[test]
|
||||
fn ensure_angle_brackets_wraps_bare_id() {
|
||||
assert_eq!(ensure_angle_brackets("abc@host"), "<abc@host>");
|
||||
assert_eq!(ensure_angle_brackets(" abc@host "), "<abc@host>");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_angle_brackets_preserves_wrapped_id() {
|
||||
assert_eq!(ensure_angle_brackets("<abc@host>"), "<abc@host>");
|
||||
assert_eq!(ensure_angle_brackets(" <abc@host> "), "<abc@host>");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ensure_angle_brackets_handles_partial() {
|
||||
// One-sided wrap: code path is intentional — wraps the trimmed
|
||||
// input again, which produces a doubled bracket. We're not the
|
||||
// RFC-5322 parser; downstream lettre catches truly malformed
|
||||
// values. This is the "best-effort" surface.
|
||||
assert_eq!(ensure_angle_brackets("<abc@host"), "<<abc@host>");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,17 +27,29 @@ use crate::{imap as imap_mod, smtp as smtp_mod};
|
|||
|
||||
#[derive(Clone)]
|
||||
pub struct MailService {
|
||||
inner: Arc<MailInner>,
|
||||
}
|
||||
|
||||
struct MailInner {
|
||||
config: Config,
|
||||
config: Arc<Config>,
|
||||
}
|
||||
|
||||
impl MailService {
|
||||
pub fn new(config: Config) -> Self {
|
||||
Self {
|
||||
inner: Arc::new(MailInner { config }),
|
||||
config: Arc::new(config),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Convert any `anyhow::Result<T>` into the `Result<String, String>` shape
|
||||
/// rmcp tool methods must return. Renders the error chain via `{:#}` so the
|
||||
/// `with_context` causes show up in the MCP `isError` content.
|
||||
trait IntoMcpError<T> {
|
||||
fn to_mcp(self) -> Result<String, String>;
|
||||
}
|
||||
|
||||
impl<T: serde::Serialize> IntoMcpError<T> for Result<T, anyhow::Error> {
|
||||
fn to_mcp(self) -> Result<String, String> {
|
||||
match self {
|
||||
Ok(v) => serde_json::to_string(&v).map_err(|e| format!("serialize: {e:#}")),
|
||||
Err(e) => Err(format!("{e:#}")),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -259,7 +271,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: SendArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -282,14 +293,7 @@ impl MailService {
|
|||
in_reply_to: args.in_reply_to,
|
||||
references: args.references,
|
||||
};
|
||||
let out = smtp_mod::send(account, input)
|
||||
.await
|
||||
.map_err(|e| format!("{e:#}"))?;
|
||||
serde_json::to_string(&serde_json::json!({
|
||||
"message_id": out.message_id,
|
||||
"sent_at": out.sent_at,
|
||||
}))
|
||||
.map_err(|e| e.to_string())
|
||||
smtp_mod::send(account, input).await.to_mcp()
|
||||
}
|
||||
|
||||
#[tool(
|
||||
|
|
@ -301,7 +305,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: ListArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -329,7 +332,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: FolderListArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -348,7 +350,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: SearchArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -373,7 +374,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: ThreadArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -397,7 +397,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: MoveArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -421,7 +420,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: MarkArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -441,7 +439,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: AttachmentGetArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -465,7 +462,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: ReplyArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -485,7 +481,11 @@ impl MailService {
|
|||
let to: Vec<String> = if let Some(overrides) = args.to_override {
|
||||
overrides
|
||||
} else {
|
||||
original.from.iter().map(|s| extract_addr(s)).collect()
|
||||
original
|
||||
.from
|
||||
.iter()
|
||||
.map(|s| smtp_mod::extract_bare_addr(s))
|
||||
.collect()
|
||||
};
|
||||
if to.is_empty() {
|
||||
return Err("no recipient — original has no From and no to_override given".into());
|
||||
|
|
@ -493,42 +493,49 @@ impl MailService {
|
|||
|
||||
// Cc: reply_all -> original.cc; else empty.
|
||||
let cc: Vec<String> = if args.reply_all {
|
||||
original.cc.iter().map(|s| extract_addr(s)).collect()
|
||||
original
|
||||
.cc
|
||||
.iter()
|
||||
.map(|s| smtp_mod::extract_bare_addr(s))
|
||||
.collect()
|
||||
} else {
|
||||
vec![]
|
||||
};
|
||||
|
||||
// Subject: keep an existing Re: prefix; otherwise add one.
|
||||
let subject = if original
|
||||
// Subject: keep an existing Re: prefix; otherwise add one. No alloc
|
||||
// for the check — slice the first 3 bytes and compare case-insensitive.
|
||||
let has_re_prefix = original
|
||||
.subject
|
||||
.to_ascii_lowercase()
|
||||
.starts_with("re:")
|
||||
{
|
||||
.get(..3)
|
||||
.map(|s| s.eq_ignore_ascii_case("re:"))
|
||||
.unwrap_or(false);
|
||||
let subject = if has_re_prefix {
|
||||
original.subject.clone()
|
||||
} else {
|
||||
format!("Re: {}", original.subject)
|
||||
};
|
||||
|
||||
// Threading headers: In-Reply-To = parent Message-Id; References =
|
||||
// parent References + parent Message-Id.
|
||||
let parent_msgid = original
|
||||
// parent References + parent Message-Id. mail-parser strips angle
|
||||
// brackets when it returns Message-Id; lettre passes through verbatim
|
||||
// and strict RFC-5322 receivers will drop the threading link if the
|
||||
// brackets are missing. Force the canonical form before sending.
|
||||
let parent_msgid = smtp_mod::ensure_angle_brackets(
|
||||
&original
|
||||
.message_id
|
||||
.clone()
|
||||
.ok_or("original message has no Message-Id — cannot thread reply")?;
|
||||
.ok_or("original message has no Message-Id — cannot thread reply")?,
|
||||
);
|
||||
|
||||
let mut references: Vec<String> = Vec::new();
|
||||
// Mail-parser returns the References header (if any) via raw_header.
|
||||
// Read it out of the flat headers map we already serialized.
|
||||
if let Some(refs_value) = original.headers.get("References") {
|
||||
if let Some(refs_str) = refs_value.as_str() {
|
||||
if let Some(refs_str) = original.headers.get("References") {
|
||||
references.extend(
|
||||
refs_str
|
||||
.split_whitespace()
|
||||
.filter(|s| !s.is_empty())
|
||||
.map(|s| s.to_string()),
|
||||
.map(smtp_mod::ensure_angle_brackets),
|
||||
);
|
||||
}
|
||||
}
|
||||
references.push(parent_msgid.clone());
|
||||
|
||||
let input = smtp_mod::SendInput {
|
||||
|
|
@ -550,14 +557,7 @@ impl MailService {
|
|||
in_reply_to: Some(parent_msgid),
|
||||
references,
|
||||
};
|
||||
let out = smtp_mod::send(account, input)
|
||||
.await
|
||||
.map_err(|e| format!("{e:#}"))?;
|
||||
serde_json::to_string(&serde_json::json!({
|
||||
"message_id": out.message_id,
|
||||
"sent_at": out.sent_at,
|
||||
}))
|
||||
.map_err(|e| e.to_string())
|
||||
smtp_mod::send(account, input).await.to_mcp()
|
||||
}
|
||||
|
||||
#[tool(
|
||||
|
|
@ -569,7 +569,6 @@ impl MailService {
|
|||
#[tool(aggr)] args: ReadArgs,
|
||||
) -> Result<String, String> {
|
||||
let account = self
|
||||
.inner
|
||||
.config
|
||||
.account(args.account.as_deref())
|
||||
.map_err(|e| e.to_string())?;
|
||||
|
|
@ -585,57 +584,17 @@ impl MailService {
|
|||
}
|
||||
}
|
||||
|
||||
/// Strip an RFC-5322 mailbox down to the bare address, dropping any display
|
||||
/// name. `Kayos <kayos@sulkta.com>` -> `kayos@sulkta.com`; `kayos@sulkta.com`
|
||||
/// passes through unchanged. Used by `mail_reply` because lettre's address
|
||||
/// parser wants either a bare addr or a full `"Name" <addr>` shape.
|
||||
fn extract_addr(s: &str) -> String {
|
||||
if let (Some(lt), Some(gt)) = (s.find('<'), s.rfind('>')) {
|
||||
if lt < gt {
|
||||
return s[lt + 1..gt].trim().to_string();
|
||||
}
|
||||
}
|
||||
s.trim().to_string()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn extract_addr_strips_display_name() {
|
||||
assert_eq!(extract_addr("Kayos <kayos@sulkta.com>"), "kayos@sulkta.com");
|
||||
assert_eq!(
|
||||
extract_addr("\"Cobb Hayes\" <cobb@sulkta.com>"),
|
||||
"cobb@sulkta.com"
|
||||
);
|
||||
assert_eq!(extract_addr(" spaces <a@b.com> "), "a@b.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_addr_passes_bare_address() {
|
||||
assert_eq!(extract_addr("plain@addr.com"), "plain@addr.com");
|
||||
assert_eq!(extract_addr(" trim-me@x.com "), "trim-me@x.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn extract_addr_handles_garbage_gracefully() {
|
||||
// Reversed brackets — fall through to trimmed input
|
||||
assert_eq!(extract_addr("> a@b.com <"), "> a@b.com <");
|
||||
// No brackets and no address — caller of extract_addr is
|
||||
// responsible; we just don't panic.
|
||||
assert_eq!(extract_addr(""), "");
|
||||
assert_eq!(extract_addr("Just A Name"), "Just A Name");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn to_field_collapses_to_vec() {
|
||||
match (ToField::One("a@b.com".into())).into_vec() {
|
||||
v => assert_eq!(v, vec!["a@b.com".to_string()]),
|
||||
}
|
||||
match (ToField::Many(vec!["a@b.com".into(), "c@d.com".into()])).into_vec() {
|
||||
v => assert_eq!(v, vec!["a@b.com".to_string(), "c@d.com".to_string()]),
|
||||
}
|
||||
let v = ToField::One("a@b.com".into()).into_vec();
|
||||
assert_eq!(v, vec!["a@b.com".to_string()]);
|
||||
|
||||
let v = ToField::Many(vec!["a@b.com".into(), "c@d.com".into()]).into_vec();
|
||||
assert_eq!(v, vec!["a@b.com".to_string(), "c@d.com".to_string()]);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue