audit-fix round 3: LOW-1 mime cleanup, INFO-2 drop empty snippet, INFO-3 unit tests + format_imap_since tightening

- LOW-1: mime_type construction simplified. Single `content_type().map()`
  with proper fallback instead of two unwrap_or chains where the second
  default could never fire.
- INFO-2: ListEntry.snippet field dropped. Was always an empty string
  because list mode doesn't fetch the body. Field stays out until /
  unless we add a partial-body fetch in Phase C.
- INFO-3: 18 unit tests for the pure validation helpers — validate_mailbox
  (accept + reject CR/LF/NUL/quote/backslash), has_imap_literal (with /
  without digits), format_imap_since (canonical + bad-shape rejection),
  strip_msgid_braces, clamp_limit, render_flag (every variant +
  Custom), strip_quotes (matched / unmatched / inner / empties),
  civil_from_unix (epoch / Y2K / 2026-05-21 / pre-epoch / leap day).
- Bonus catch from the test suite: format_imap_since accepted
  malformed shapes like "21-05-2026" (parsed as y=21 m=5 d=2026)
  and "2026-5-21" (no field-width check). Added 4-2-2 digit-width
  check + year range (1900..=9999) + day range (1..=31). Month range
  was already enforced.

All 18 tests pass.
This commit is contained in:
Kayos 2026-05-21 08:00:50 -07:00
parent 54a1a6bf22
commit 6fb63b0ca0
3 changed files with 198 additions and 15 deletions

View file

@ -167,3 +167,43 @@ fn config_path() -> Result<PathBuf> {
.ok_or_else(|| anyhow!("could not resolve $XDG_CONFIG_HOME / ~/.config"))?;
Ok(home.join("mail-mcp").join("config.toml"))
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn strip_quotes_strips_matched_pairs() {
assert_eq!(strip_quotes(r#""hello""#), "hello");
assert_eq!(strip_quotes("'hello'"), "hello");
}
#[test]
fn strip_quotes_leaves_unmatched_intact() {
// Unbalanced — the asymmetric strip bug from HIGH-3. These must
// pass through unchanged so a password starting with `"` keeps it.
assert_eq!(strip_quotes(r#""hello"#), r#""hello"#);
assert_eq!(strip_quotes(r#"hello""#), r#"hello""#);
assert_eq!(strip_quotes("'hello"), "'hello");
assert_eq!(strip_quotes("hello'"), "hello'");
// Mixed quotes — also unchanged.
assert_eq!(strip_quotes(r#""hello'"#), r#""hello'"#);
assert_eq!(strip_quotes(r#"'hello""#), r#"'hello""#);
}
#[test]
fn strip_quotes_handles_empties_and_singletons() {
assert_eq!(strip_quotes(""), "");
assert_eq!(strip_quotes(r#"""#), r#"""#); // single char — no pair
assert_eq!(strip_quotes("'"), "'");
// Single quote with same on both sides of nothing == empty.
assert_eq!(strip_quotes(r#""""#), "");
assert_eq!(strip_quotes("''"), "");
}
#[test]
fn strip_quotes_does_not_eat_inner_quotes() {
assert_eq!(strip_quotes(r#""he"llo""#), r#"he"llo"#);
assert_eq!(strip_quotes(r#""he\"llo""#), r#"he\"llo"#);
}
}

View file

@ -51,7 +51,6 @@ pub struct ListEntry {
pub to: Vec<String>,
pub subject: String,
pub date: Option<String>,
pub snippet: String,
pub has_attachments: bool,
pub flags: Vec<String>,
}
@ -219,10 +218,6 @@ fn fetch_to_list_entry(msg: &Fetch) -> ListEntry {
(vec![], vec![], String::new(), None, None)
};
// We didn't fetch the body for the list view — snippet stays empty.
// (read() fetches the body separately.)
let snippet = String::new();
// has_attachments is best-guessed from Content-Type in the header
// block, since we don't pull the body. multipart/mixed almost always
// means attachments are present.
@ -243,7 +238,6 @@ fn fetch_to_list_entry(msg: &Fetch) -> ListEntry {
to,
subject,
date,
snippet,
has_attachments,
flags,
}
@ -340,15 +334,16 @@ pub async fn read(
.attachments()
.map(|att| AttachmentMeta {
filename: att.attachment_name().unwrap_or("attachment").to_string(),
mime_type: format!(
"{}/{}",
att.content_type()
.map(|ct| ct.ctype().to_string())
.unwrap_or_else(|| "application".into()),
att.content_type()
.and_then(|ct| ct.subtype().map(|s| s.to_string()))
.unwrap_or_else(|| "octet-stream".into()),
),
mime_type: att
.content_type()
.map(|ct| {
format!(
"{}/{}",
ct.ctype(),
ct.subtype().unwrap_or("octet-stream")
)
})
.unwrap_or_else(|| "application/octet-stream".into()),
size: att.contents().len(),
})
.collect();
@ -656,9 +651,20 @@ fn format_imap_since(iso_date: &str) -> Result<String> {
if parts.len() != 3 {
return Err(anyhow!("expected YYYY-MM-DD"));
}
// Field-width sanity: YYYY=4, MM=2, DD=2. Catches "21-05-2026" and
// "2026-5-21" before we ever try to substitute them into IMAP wire bytes.
if parts[0].len() != 4 || parts[1].len() != 2 || parts[2].len() != 2 {
return Err(anyhow!("expected YYYY-MM-DD with 4-2-2 digit widths"));
}
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 !(1..=31).contains(&d) {
return Err(anyhow!("day out of range (1..=31)"));
}
let mon = match m {
1 => "Jan",
2 => "Feb",
@ -737,3 +743,101 @@ fn rustls_roots() -> rustls::RootCertStore {
roots.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned());
roots
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn validate_mailbox_accepts_normal_names() {
assert!(validate_mailbox("INBOX").is_ok());
assert!(validate_mailbox("Sent").is_ok());
assert!(validate_mailbox("Drafts").is_ok());
assert!(validate_mailbox("Personal/Family").is_ok());
assert!(validate_mailbox("client.alice").is_ok());
assert!(validate_mailbox("INBOX with spaces").is_ok());
}
#[test]
fn validate_mailbox_rejects_injection_bytes() {
assert!(validate_mailbox("").is_err());
assert!(validate_mailbox("INBOX\r\nLOGOUT").is_err());
assert!(validate_mailbox("INBOX\nfoo").is_err());
assert!(validate_mailbox("INBOX\rbar").is_err());
assert!(validate_mailbox("INBOX\0null").is_err());
assert!(validate_mailbox("INBOX\"quote").is_err());
assert!(validate_mailbox("INBOX\\backslash").is_err());
assert!(validate_mailbox("\"wholly-quoted\"").is_err());
}
#[test]
fn has_imap_literal_catches_braces_with_digits() {
assert!(has_imap_literal("SUBJECT {5}hello"));
assert!(has_imap_literal("{0}"));
assert!(has_imap_literal("{123+}\r\nfoo"));
assert!(has_imap_literal("HEADER \"X\" {7+}"));
}
#[test]
fn has_imap_literal_ignores_braces_without_digits() {
assert!(!has_imap_literal("SUBJECT \"foo\""));
assert!(!has_imap_literal("{}")); // empty braces — not a literal opener
assert!(!has_imap_literal("{abc}")); // word-style braces
assert!(!has_imap_literal("a{b}c"));
assert!(!has_imap_literal(""));
}
#[test]
fn format_imap_since_canonical() {
assert_eq!(format_imap_since("2026-05-21").unwrap(), "21-May-2026");
assert_eq!(format_imap_since("2026-01-01").unwrap(), "01-Jan-2026");
assert_eq!(format_imap_since("2026-12-31").unwrap(), "31-Dec-2026");
}
#[test]
fn format_imap_since_rejects_bad_shape() {
assert!(format_imap_since("21-05-2026").is_err()); // wrong order
assert!(format_imap_since("2026/05/21").is_err()); // wrong separator
assert!(format_imap_since("2026-13-01").is_err()); // month out of range
assert!(format_imap_since("not-a-date").is_err());
assert!(format_imap_since("").is_err());
assert!(format_imap_since("-2026-05-21").is_err()); // leading negative
}
#[test]
fn strip_msgid_braces_handles_both_shapes() {
assert_eq!(strip_msgid_braces("<abc@host>"), "abc@host");
assert_eq!(strip_msgid_braces("abc@host"), "abc@host");
assert_eq!(strip_msgid_braces(" <abc@host> "), "abc@host");
// Asymmetric brackets: leave each strip independent (matches code).
assert_eq!(strip_msgid_braces("<abc@host"), "abc@host");
assert_eq!(strip_msgid_braces("abc@host>"), "abc@host");
}
#[test]
fn clamp_limit_treats_zero_and_none_as_default() {
assert_eq!(clamp_limit(None), DEFAULT_LIMIT);
assert_eq!(clamp_limit(Some(0)), DEFAULT_LIMIT);
assert_eq!(clamp_limit(Some(1)), 1);
assert_eq!(clamp_limit(Some(50)), 50);
assert_eq!(clamp_limit(Some(MAX_LIMIT)), MAX_LIMIT);
assert_eq!(clamp_limit(Some(MAX_LIMIT + 1)), MAX_LIMIT);
assert_eq!(clamp_limit(Some(u32::MAX)), MAX_LIMIT);
}
#[test]
fn render_flag_canonical() {
use std::borrow::Cow;
assert_eq!(render_flag(&Flag::Seen), "\\Seen");
assert_eq!(render_flag(&Flag::Answered), "\\Answered");
assert_eq!(render_flag(&Flag::Flagged), "\\Flagged");
assert_eq!(render_flag(&Flag::Deleted), "\\Deleted");
assert_eq!(render_flag(&Flag::Draft), "\\Draft");
assert_eq!(render_flag(&Flag::Recent), "\\Recent");
assert_eq!(render_flag(&Flag::MayCreate), "\\MayCreate");
assert_eq!(
render_flag(&Flag::Custom(Cow::Borrowed("$Important"))),
"$Important"
);
}
}

View file

@ -282,3 +282,42 @@ fn civil_from_unix(t: i64) -> (i64, u32, u32, u32, u32, u32) {
(y, m, d, hour, minute, second)
}
#[cfg(test)]
mod tests {
use super::*;
#[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
assert_eq!(
civil_from_unix(1_779_371_043),
(2026, 5, 21, 13, 44, 3)
);
}
#[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));
}
#[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));
}
}