final-approval audit fixes: HIGH-1/2/3

Three findings from the post-cleanup approval audit, all blockers
before the rename to a real codename:

HIGH-1: ReadOutput.headers map kept LAST occurrence of duplicate
headers, not FIRST. Comment said 'keep the first occurrence' but the
code used Message::header_raw(name) which internally does
.iter().rev().find(...) — returns the last one. For load-bearing
headers like References this is usually singular so the bug was
latent, but an attacker who could inject a second References: line
would have gotten to override the first one used by mail_reply for
threading. Switched to parsed.headers_raw() which iterates in arrival
order — first-occurrence guaranteed.

HIGH-2: tokio-rustls default features pulled aws-lc-rs + aws-lc-sys
into the dep tree even though we explicitly went ring-only on rustls.
The default feature chain on tokio-rustls v0.26 enables 'aws_lc_rs'
via rustls. Pinned tokio-rustls to default-features=false and the
matching small feature set: logging, tls12, ring. Verified via
`cargo tree` — no aws-lc-* in the build, single ring v0.17.14
shared between rustls + tokio-rustls. ~9s shorter cmake step in cold
builds, smaller binary, no C-FFI crypto surface area.

HIGH-3: IntoMcpError trait was introduced in the cleanup pass but
applied at only 2 of 10 tools — the other 8 still used the manual
.map_err(|e| format!('{e:#}'))? + serde_json::to_string chain.
Maintenance trap. Applied to_mcp() at all 8 sites
(mail_inbox_list, mail_folder_list, mail_search, mail_thread,
mail_attachment_get, mail_inbox_read; mail_move and mail_mark stay
with literal {"ok":true} returns — no value to serialize). Tool
methods are now uniformly:
    imap_mod::xxx(...).await.to_mcp()
or for the few that need pre-arg work, three lines instead of seven.

Wire smoke verified — read on uid 34 returns the same 13 headers
shape, no empties, all canonical fields populated. cargo test 31/31.

Repo chain: 2240bf7 -> 4251f51 -> f4b3199 -> 6432a1f -> 54a1a6b ->
6fb63b0 -> f7e698b -> b681953 -> 7c8e246 -> this.
This commit is contained in:
Kayos 2026-05-21 09:22:39 -07:00
parent 7c8e246544
commit 5e1c63eeaa
4 changed files with 30 additions and 112 deletions

77
Cargo.lock generated
View file

@ -107,28 +107,6 @@ version = "1.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8"
[[package]]
name = "aws-lc-rs"
version = "1.17.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "5ec2f1fc3ec205783a5da9a7e6c1509cc69dedf09a1949e412c1e18469326d00"
dependencies = [
"aws-lc-sys",
"zeroize",
]
[[package]]
name = "aws-lc-sys"
version = "0.41.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1a2f9779ce85b93ab6170dd940ad0169b5766ff848247aff13bb788b832fe3f4"
dependencies = [
"cc",
"cmake",
"dunce",
"fs_extra",
]
[[package]] [[package]]
name = "base64" name = "base64"
version = "0.21.7" version = "0.21.7"
@ -166,8 +144,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a1dce859f0832a7d088c4f1119888ab94ef4b5d6795d1ce05afb7fe159d79f98" checksum = "a1dce859f0832a7d088c4f1119888ab94ef4b5d6795d1ce05afb7fe159d79f98"
dependencies = [ dependencies = [
"find-msvc-tools", "find-msvc-tools",
"jobserver",
"libc",
"shlex", "shlex",
] ]
@ -191,15 +167,6 @@ dependencies = [
"windows-link", "windows-link",
] ]
[[package]]
name = "cmake"
version = "0.1.58"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c0f78a02292a74a88ac736019ab962ece0bc380e3f977bf72e376c5d78ff0678"
dependencies = [
"cc",
]
[[package]] [[package]]
name = "compression-codecs" name = "compression-codecs"
version = "0.4.38" version = "0.4.38"
@ -299,12 +266,6 @@ dependencies = [
"syn", "syn",
] ]
[[package]]
name = "dunce"
version = "1.0.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "92773504d58c093f6de2459af4af33faa518c13451eb8f2b5698ed3d36e7c813"
[[package]] [[package]]
name = "dyn-clone" name = "dyn-clone"
version = "1.0.20" version = "1.0.20"
@ -416,12 +377,6 @@ dependencies = [
"percent-encoding", "percent-encoding",
] ]
[[package]]
name = "fs_extra"
version = "1.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "42703706b716c37f96a77aea830392ad231f44c9e9a67872fa5548707e11b11c"
[[package]] [[package]]
name = "futures" name = "futures"
version = "0.3.32" version = "0.3.32"
@ -521,18 +476,6 @@ dependencies = [
"wasi", "wasi",
] ]
[[package]]
name = "getrandom"
version = "0.3.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd"
dependencies = [
"cfg-if",
"libc",
"r-efi 5.3.0",
"wasip2",
]
[[package]] [[package]]
name = "getrandom" name = "getrandom"
version = "0.4.2" version = "0.4.2"
@ -541,7 +484,7 @@ checksum = "0de51e6874e94e7bf76d726fc5d13ba782deca734ff60d5bb2fb2607c7406555"
dependencies = [ dependencies = [
"cfg-if", "cfg-if",
"libc", "libc",
"r-efi 6.0.0", "r-efi",
"wasip2", "wasip2",
"wasip3", "wasip3",
] ]
@ -733,16 +676,6 @@ version = "1.0.18"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682" checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682"
[[package]]
name = "jobserver"
version = "0.1.34"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9afb3de4395d6b3e67a780b6de64b51c978ecf11cb9a462c66be7d4ca9039d33"
dependencies = [
"getrandom 0.3.4",
"libc",
]
[[package]] [[package]]
name = "js-sys" name = "js-sys"
version = "0.3.98" version = "0.3.98"
@ -1080,12 +1013,6 @@ version = "0.5.2"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "478e0585659a122aa407eb7e3c0e1fa51b1d8a870038bd29f0cf4a8551eea972" checksum = "478e0585659a122aa407eb7e3c0e1fa51b1d8a870038bd29f0cf4a8551eea972"
[[package]]
name = "r-efi"
version = "5.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f"
[[package]] [[package]]
name = "r-efi" name = "r-efi"
version = "6.0.0" version = "6.0.0"
@ -1192,7 +1119,6 @@ version = "0.23.40"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b"
dependencies = [ dependencies = [
"aws-lc-rs",
"log", "log",
"once_cell", "once_cell",
"ring", "ring",
@ -1217,7 +1143,6 @@ version = "0.103.13"
source = "registry+https://github.com/rust-lang/crates.io-index" source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e"
dependencies = [ dependencies = [
"aws-lc-rs",
"ring", "ring",
"rustls-pki-types", "rustls-pki-types",
"untrusted", "untrusted",

View file

@ -37,7 +37,10 @@ lettre = { version = "0.11", default-features = false, features = [
# IMAP — async-imap is tokio-native and supports UID-based addressing # IMAP — async-imap is tokio-native and supports UID-based addressing
# (which we use throughout the API surface). # (which we use throughout the API surface).
async-imap = { version = "0.10", default-features = false, features = ["runtime-tokio"] } async-imap = { version = "0.10", default-features = false, features = ["runtime-tokio"] }
tokio-rustls = "0.26" # tokio-rustls default-features pulls in aws-lc-rs via rustls's default
# feature chain. We use `ring` exclusively (installed once in main.rs);
# turn off defaults and add back only the small pieces we want.
tokio-rustls = { version = "0.26", default-features = false, features = ["logging", "tls12", "ring"] }
rustls = { version = "0.23", default-features = false, features = ["std", "tls12", "ring"] } rustls = { version = "0.23", default-features = false, features = ["std", "tls12", "ring"] }
rustls-pki-types = "1" rustls-pki-types = "1"
webpki-roots = "0.26" webpki-roots = "0.26"

View file

@ -386,25 +386,25 @@ pub async fn read(
}) })
.collect(); .collect();
// Headers as a flat name→raw-value map. mail-parser's `HeaderValue:: // Flat name→raw-value map.
// as_text()` returns None for structured variants (Address / DateTime / //
// ContentType / Received), so we use `Message::header_raw(name)` to // mail-parser's `headers_raw()` returns `(name, raw_value)` pairs in
// get the un-decoded header value as &str — uniform across all // ARRIVAL order — we use that directly so on duplicate headers we
// header types. Empty-valued headers are skipped (would surface as // keep the FIRST occurrence (matching the comment). `header_raw(name)`
// `""` entries the caller has to filter anyway). // looks the wrong way for this purpose: internally it does
// `iter().rev().find(...)` and returns the LAST instance, which would
// let an attacker who can inject a second `References:` line override
// the first one. The iterator avoids that footgun.
let mut headers: BTreeMap<String, String> = BTreeMap::new(); let mut headers: BTreeMap<String, String> = BTreeMap::new();
for h in parsed.headers() { for (name, raw) in parsed.headers_raw() {
let name = h.name();
if headers.contains_key(name) { if headers.contains_key(name) {
continue; // duplicate header — keep the first occurrence continue;
} }
if let Some(raw) = parsed.header_raw(name) {
let trimmed = raw.trim(); let trimmed = raw.trim();
if !trimmed.is_empty() { if !trimmed.is_empty() {
headers.insert(name.to_string(), trimmed.to_string()); headers.insert(name.to_string(), trimmed.to_string());
} }
} }
}
let subject = parsed.subject().unwrap_or_default().to_string(); let subject = parsed.subject().unwrap_or_default().to_string();

View file

@ -308,7 +308,7 @@ impl MailService {
.config .config
.account(args.account.as_deref()) .account(args.account.as_deref())
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let entries = imap_mod::list( imap_mod::list(
account, account,
imap_mod::ListOpts { imap_mod::ListOpts {
since: args.since, since: args.since,
@ -317,10 +317,8 @@ impl MailService {
folder: args.folder, folder: args.folder,
}, },
) )
.await .await
.map_err(|e| format!("{e:#}"))?; .to_mcp()
serde_json::to_string(&entries).map_err(|e| e.to_string())
} }
#[tool( #[tool(
@ -335,10 +333,7 @@ impl MailService {
.config .config
.account(args.account.as_deref()) .account(args.account.as_deref())
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let folders = imap_mod::list_folders(account) imap_mod::list_folders(account).await.to_mcp()
.await
.map_err(|e| format!("{e:#}"))?;
serde_json::to_string(&folders).map_err(|e| e.to_string())
} }
#[tool( #[tool(
@ -353,16 +348,14 @@ impl MailService {
.config .config
.account(args.account.as_deref()) .account(args.account.as_deref())
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let entries = imap_mod::search( imap_mod::search(
account, account,
&args.query, &args.query,
args.folder.as_deref(), args.folder.as_deref(),
args.limit, args.limit,
) )
.await .await
.map_err(|e| format!("{e:#}"))?; .to_mcp()
serde_json::to_string(&entries).map_err(|e| e.to_string())
} }
#[tool( #[tool(
@ -377,15 +370,14 @@ impl MailService {
.config .config
.account(args.account.as_deref()) .account(args.account.as_deref())
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let entries = imap_mod::thread( imap_mod::thread(
account, account,
&args.message_id, &args.message_id,
args.folder.as_deref(), args.folder.as_deref(),
args.limit, args.limit,
) )
.await .await
.map_err(|e| format!("{e:#}"))?; .to_mcp()
serde_json::to_string(&entries).map_err(|e| e.to_string())
} }
#[tool( #[tool(
@ -442,15 +434,14 @@ impl MailService {
.config .config
.account(args.account.as_deref()) .account(args.account.as_deref())
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let out = imap_mod::attachment_get( imap_mod::attachment_get(
account, account,
args.uid, args.uid,
args.folder.as_deref(), args.folder.as_deref(),
args.attachment_index as usize, args.attachment_index as usize,
) )
.await .await
.map_err(|e| format!("{e:#}"))?; .to_mcp()
serde_json::to_string(&out).map_err(|e| e.to_string())
} }
#[tool( #[tool(
@ -572,15 +563,14 @@ impl MailService {
.config .config
.account(args.account.as_deref()) .account(args.account.as_deref())
.map_err(|e| e.to_string())?; .map_err(|e| e.to_string())?;
let out = imap_mod::read( imap_mod::read(
account, account,
args.uid, args.uid,
args.folder.as_deref(), args.folder.as_deref(),
args.format.as_deref().unwrap_or("text"), args.format.as_deref().unwrap_or("text"),
) )
.await .await
.map_err(|e| format!("{e:#}"))?; .to_mcp()
serde_json::to_string(&out).map_err(|e| e.to_string())
} }
} }