audit-fix round 2: LOW-3, LOW-5, LOW-6
- LOW-3 (canonical Flag display): render_flag() pattern-matches the
async-imap Flag enum to its IMAP wire syntax — \\Seen, \\Flagged,
\\Deleted, etc. — instead of Debug syntax ("Seen", Custom("...")).
Consumers checking for \\Seen now match.
- LOW-5 (schema 0=default sentinel): limit fields are now Option<u32>
instead of bare u32 with a 0-means-default contract. JSON schema
output is clearer; clamp_limit() still treats Some(0) the same as
None for backwards compatibility.
- LOW-6 (config chmod gate): Config::load() now refuses to read a
config file with group/other read bits set. Same posture as
ssh-keygen rejecting loose private-key permissions. Refuses 0644
cleanly; accepts 0600. Unix-only — Windows path is a no-op.
Smoke verified: loose-chmod test refuses to start with the expected
error; tight-chmod test starts and serves initialize cleanly. All
seven tools still listed with valid input schemas.
This commit is contained in:
parent
f4b3199e86
commit
6432a1f5ff
3 changed files with 69 additions and 26 deletions
|
|
@ -47,6 +47,7 @@ fn default_true() -> bool {
|
|||
impl Config {
|
||||
pub fn load() -> Result<Self> {
|
||||
let path = config_path()?;
|
||||
check_chmod(&path)?;
|
||||
let text = std::fs::read_to_string(&path)
|
||||
.with_context(|| format!("read config {}", path.display()))?;
|
||||
let cfg: Self = toml::from_str(&text)
|
||||
|
|
@ -133,6 +134,31 @@ fn strip_quotes(s: &str) -> &str {
|
|||
s
|
||||
}
|
||||
|
||||
/// Defense-in-depth chmod check on the config file. Refuse to load anything
|
||||
/// readable by group or other — the config carries password_env / password_file
|
||||
/// pointers; even paths-to-secrets shouldn't be world-readable. Same posture
|
||||
/// as ssh-keygen complaining about loose permissions on a private key.
|
||||
#[cfg(unix)]
|
||||
fn check_chmod(path: &std::path::Path) -> Result<()> {
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let meta = std::fs::metadata(path)
|
||||
.with_context(|| format!("stat config {}", path.display()))?;
|
||||
let mode = meta.permissions().mode() & 0o777;
|
||||
if mode & 0o077 != 0 {
|
||||
return Err(anyhow!(
|
||||
"config {} has loose permissions {:o} — must be 0600 (group/other bits not allowed)",
|
||||
path.display(),
|
||||
mode
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[cfg(not(unix))]
|
||||
fn check_chmod(_path: &std::path::Path) -> Result<()> {
|
||||
Ok(()) // Windows / other: skip — no comparable concept.
|
||||
}
|
||||
|
||||
fn config_path() -> Result<PathBuf> {
|
||||
if let Ok(p) = std::env::var("MAIL_MCP_CONFIG") {
|
||||
return Ok(PathBuf::from(shellexpand::tilde(&p).into_owned()));
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@
|
|||
use std::sync::Arc;
|
||||
|
||||
use anyhow::{anyhow, Context, Result};
|
||||
use async_imap::types::Fetch;
|
||||
use async_imap::types::{Fetch, Flag};
|
||||
use futures::StreamExt;
|
||||
use mail_parser::{MessageParser, MimeHeaders};
|
||||
use rustls::pki_types::ServerName;
|
||||
|
|
@ -24,10 +24,25 @@ use crate::config::Account;
|
|||
pub struct ListOpts {
|
||||
pub since: Option<String>, // YYYY-MM-DD
|
||||
pub unread_only: bool,
|
||||
pub limit: u32, // 0 means default (50)
|
||||
pub limit: Option<u32>, // None → DEFAULT_LIMIT
|
||||
pub folder: Option<String>, // None → INBOX
|
||||
}
|
||||
|
||||
/// Render an async-imap `Flag` as its canonical IMAP wire string:
|
||||
/// `\Seen`, `\Flagged`, etc., or `$Custom` / `keyword` for user flags.
|
||||
fn render_flag(f: &Flag<'_>) -> String {
|
||||
match f {
|
||||
Flag::Seen => "\\Seen".into(),
|
||||
Flag::Answered => "\\Answered".into(),
|
||||
Flag::Flagged => "\\Flagged".into(),
|
||||
Flag::Deleted => "\\Deleted".into(),
|
||||
Flag::Draft => "\\Draft".into(),
|
||||
Flag::Recent => "\\Recent".into(),
|
||||
Flag::MayCreate => "\\MayCreate".into(),
|
||||
Flag::Custom(s) => s.to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize)]
|
||||
pub struct ListEntry {
|
||||
pub uid: u32,
|
||||
|
|
@ -79,6 +94,17 @@ const MAX_LIMIT: u32 = 500;
|
|||
/// but doesn't double-buffer through `String::from_utf8_lossy`).
|
||||
const MAX_RAW_EML_BYTES: u64 = 20 * 1024 * 1024;
|
||||
|
||||
/// Clamp a caller-provided limit to `[1, MAX_LIMIT]` with `DEFAULT_LIMIT`
|
||||
/// when omitted. Returning u32 since IMAP UID truncation works on count.
|
||||
fn clamp_limit(limit: Option<u32>) -> u32 {
|
||||
match limit {
|
||||
None => DEFAULT_LIMIT,
|
||||
Some(0) => DEFAULT_LIMIT,
|
||||
Some(n) if n > MAX_LIMIT => MAX_LIMIT,
|
||||
Some(n) => n,
|
||||
}
|
||||
}
|
||||
|
||||
/// 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
|
||||
|
|
@ -111,11 +137,7 @@ fn validate_mailbox(name: &str) -> Result<()> {
|
|||
pub async fn list(account: &Account, opts: ListOpts) -> Result<Vec<ListEntry>> {
|
||||
let folder = opts.folder.as_deref().unwrap_or("INBOX");
|
||||
validate_mailbox(folder)?;
|
||||
let limit = match opts.limit {
|
||||
0 => DEFAULT_LIMIT,
|
||||
n if n > MAX_LIMIT => MAX_LIMIT,
|
||||
n => n,
|
||||
};
|
||||
let limit = clamp_limit(opts.limit);
|
||||
|
||||
let mut session = open_session(account).await?;
|
||||
session
|
||||
|
|
@ -179,7 +201,7 @@ pub async fn list(account: &Account, opts: ListOpts) -> Result<Vec<ListEntry>> {
|
|||
|
||||
fn fetch_to_list_entry(msg: &Fetch) -> ListEntry {
|
||||
let uid = msg.uid.unwrap_or(0);
|
||||
let flags: Vec<String> = msg.flags().map(|f| format!("{f:?}")).collect();
|
||||
let flags: Vec<String> = msg.flags().map(|f| render_flag(&f)).collect();
|
||||
let header_bytes = msg.header().unwrap_or(&[]);
|
||||
|
||||
let parser = MessageParser::default();
|
||||
|
|
@ -409,7 +431,7 @@ pub async fn search(
|
|||
account: &Account,
|
||||
query: &str,
|
||||
folder: Option<&str>,
|
||||
limit: u32,
|
||||
limit: Option<u32>,
|
||||
) -> Result<Vec<ListEntry>> {
|
||||
if query.trim().is_empty() {
|
||||
return Err(anyhow!("search query is empty"));
|
||||
|
|
@ -431,11 +453,7 @@ pub async fn search(
|
|||
}
|
||||
let folder = folder.unwrap_or("INBOX");
|
||||
validate_mailbox(folder)?;
|
||||
let limit = match limit {
|
||||
0 => DEFAULT_LIMIT,
|
||||
n if n > MAX_LIMIT => MAX_LIMIT,
|
||||
n => n,
|
||||
};
|
||||
let limit = clamp_limit(limit);
|
||||
|
||||
let mut session = open_session(account).await?;
|
||||
session
|
||||
|
|
@ -467,7 +485,7 @@ pub async fn thread(
|
|||
account: &Account,
|
||||
message_id: &str,
|
||||
folder: Option<&str>,
|
||||
limit: u32,
|
||||
limit: Option<u32>,
|
||||
) -> Result<Vec<ListEntry>> {
|
||||
let id_unbraced = strip_msgid_braces(message_id);
|
||||
if id_unbraced.is_empty() {
|
||||
|
|
@ -485,11 +503,7 @@ pub async fn thread(
|
|||
}
|
||||
let folder = folder.unwrap_or("INBOX");
|
||||
validate_mailbox(folder)?;
|
||||
let limit = match limit {
|
||||
0 => DEFAULT_LIMIT,
|
||||
n if n > MAX_LIMIT => MAX_LIMIT,
|
||||
n => n,
|
||||
};
|
||||
let limit = clamp_limit(limit);
|
||||
|
||||
let mut session = open_session(account).await?;
|
||||
session
|
||||
|
|
|
|||
|
|
@ -111,9 +111,10 @@ pub struct ListArgs {
|
|||
/// If true, only list messages without the \Seen flag.
|
||||
#[serde(default)]
|
||||
pub unread_only: bool,
|
||||
/// Max entries to return — default 50, max 500.
|
||||
/// Max entries to return — default 50, max 500. Omit for default; 0
|
||||
/// also means default (treated the same as None).
|
||||
#[serde(default)]
|
||||
pub limit: u32,
|
||||
pub limit: Option<u32>,
|
||||
/// IMAP folder. Default `INBOX`.
|
||||
#[serde(default)]
|
||||
pub folder: Option<String>,
|
||||
|
|
@ -136,9 +137,9 @@ pub struct SearchArgs {
|
|||
/// IMAP folder. Default `INBOX`.
|
||||
#[serde(default)]
|
||||
pub folder: Option<String>,
|
||||
/// Max entries — default 50, max 500.
|
||||
/// Max entries — default 50, max 500. Omit for default; 0 also means default.
|
||||
#[serde(default)]
|
||||
pub limit: u32,
|
||||
pub limit: Option<u32>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, schemars::JsonSchema)]
|
||||
|
|
@ -151,9 +152,9 @@ pub struct ThreadArgs {
|
|||
/// IMAP folder. Default `INBOX`.
|
||||
#[serde(default)]
|
||||
pub folder: Option<String>,
|
||||
/// Max entries — default 50, max 500.
|
||||
/// Max entries — default 50, max 500. Omit for default; 0 also means default.
|
||||
#[serde(default)]
|
||||
pub limit: u32,
|
||||
pub limit: Option<u32>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, schemars::JsonSchema)]
|
||||
|
|
@ -254,6 +255,7 @@ impl MailService {
|
|||
folder: args.folder,
|
||||
},
|
||||
)
|
||||
|
||||
.await
|
||||
.map_err(|e| format!("{e:#}"))?;
|
||||
serde_json::to_string(&entries).map_err(|e| e.to_string())
|
||||
|
|
@ -297,6 +299,7 @@ impl MailService {
|
|||
args.folder.as_deref(),
|
||||
args.limit,
|
||||
)
|
||||
|
||||
.await
|
||||
.map_err(|e| format!("{e:#}"))?;
|
||||
serde_json::to_string(&entries).map_err(|e| e.to_string())
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue