crates/nuncio-filter/src/ast.rs

@@ -17,7 +17,8 @@ pub enum FilterField {
     Header(String),
     /// Attachment presence indicator (`true`/`false`).
     HasAttachment,
-    /// Message size in bytes.
+    /// Message size in bytes: all stored body parts plus all attachment
+    /// payloads. Excludes headers and MIME framing, which are not retained.
     Size,
     /// Current folder location (e.g. `INBOX`).
     Folder,

crates/nuncio-filter/src/engine.rs

@@ -97,6 +97,42 @@ impl CompiledFilter {
         }
     }
 
+    /// Total byte size of the message content Nuncio holds: every body part
+    /// plus every attachment payload.
+    ///
+    /// This is what `WHERE size > N` compares against. Counting only the
+    /// plaintext body would report `0` for the very common HTML-only message
+    /// and would ignore attachments entirely, so a size rule would quietly
+    /// return the wrong verdict on exactly the mail it is usually written to
+    /// catch (the big ones). Both body parts are summed rather than one being
+    /// preferred: a `multipart/alternative` message genuinely carries the
+    /// plaintext *and* the HTML on the wire, so summing them tracks the real
+    /// message rather than one arbitrary view of it.
+    ///
+    /// The figure is the size of the decoded content as stored, not the exact
+    /// RFC822 octet count of the original transmission -- headers, MIME
+    /// boundaries, and transfer encoding are not counted, because the raw
+    /// octets are not retained and no stored column carries their length. It is
+    /// therefore a consistent lower bound on the wire size, never a guess: for
+    /// base64-encoded attachments the transmitted form is roughly a third
+    /// larger again. Rules are written against what the store actually has.
+    fn message_size_bytes(email: &Email) -> i64 {
+        let mut total: u64 = 0;
+        for part in [email.body_plain.as_deref(), email.body_html.as_deref()]
+            .into_iter()
+            .flatten()
+        {
+            total = total.saturating_add(part.len() as u64);
+        }
+        for attachment in &email.attachments {
+            total = total.saturating_add(attachment.content.len() as u64);
+        }
+        // Saturate rather than wrap: a size past `i64::MAX` is not a real
+        // message, and a wrapped negative would make `size > N` answer the
+        // opposite of the truth.
+        i64::try_from(total).unwrap_or(i64::MAX)
+    }
+
     fn eval_leaf(
         leaf: &ConditionLeaf,
         placed: PlacedEmail<'_>,
@@ -146,11 +182,7 @@ impl CompiledFilter {
                 }
             }
             FilterField::Size => {
-                let size = email
-                    .body_plain
-                    .as_ref()
-                    .map(|b| b.len() as i64)
-                    .unwrap_or(0);
+                let size = Self::message_size_bytes(email);
                 if let FilterValue::Number(target) = leaf.value {
                     match leaf.operator {
                         FilterOperator::Equals => size == target,
@@ -526,6 +558,92 @@ mod tests {
         assert_eq!(preview.matched_rule_name, Some("Urgent".to_string()));
     }
 
+    /// Evaluate one NSQL rule against one message, returning whether it fired.
+    fn size_rule_matches(nsql: &str, email: &Email) -> bool {
+        let rule = crate::parser::NsqlParser::parse_rule("Size Rule", 1, nsql).unwrap();
+        let engine = FilterEngine::new(vec![rule]).unwrap();
+        let (_, placement) = test_placed_email(&email.account_id, "", "inbox");
+        !engine
+            .evaluate(PlacedEmail {
+                email,
+                placement: &placement,
+            })
+            .is_empty()
+    }
+
+    /// `size` must reflect every body part and every attachment, not the
+    /// plaintext body alone. An HTML-only message has no plaintext body at
+    /// all, so it used to measure zero bytes and silently fell on the wrong
+    /// side of every size comparison -- `size > 100` said no for a 400-byte
+    /// message, and `size < 100` said yes.
+    #[test]
+    fn size_counts_html_body_of_html_only_message() {
+        let (mut email, _) = test_placed_email("acct-1", "HTML only", "inbox");
+        email.body_plain = None;
+        email.body_html = Some(format!("<p>{}</p>", "x".repeat(400)));
+
+        assert!(
+            size_rule_matches("WHERE size > 100 ACTION MARK READ", &email),
+            "a 407-byte HTML-only message must satisfy size > 100"
+        );
+        assert!(
+            !size_rule_matches("WHERE size < 100 ACTION MARK READ", &email),
+            "a 407-byte HTML-only message must not satisfy size < 100"
+        );
+    }
+
+    /// Attachment payloads are part of the message's size. A short note
+    /// carrying a large attachment is exactly the mail a size rule is written
+    /// to catch, and counting only the note's text missed it entirely.
+    #[test]
+    fn size_counts_attachment_payloads() {
+        let (mut email, _) = test_placed_email("acct-1", "Invoice", "inbox");
+        email.body_plain = Some("See attached.".to_string());
+        email.attachments = vec![nuncio_core::model::Attachment {
+            filename: "invoice.pdf".to_string(),
+            mime_type: "application/pdf".to_string(),
+            content: vec![0u8; 5_000].into(),
+        }];
+
+        assert!(
+            size_rule_matches("WHERE size > 4096 ACTION MARK READ", &email),
+            "a 13-byte note with a 5000-byte attachment must satisfy size > 4096"
+        );
+    }
+
+    /// A `multipart/alternative` message carries both renderings on the wire,
+    /// so both are counted rather than one being preferred -- this pins the
+    /// definition `size` is documented to have.
+    #[test]
+    fn size_sums_both_body_parts_and_attachments() {
+        let (mut email, _) = test_placed_email("acct-1", "Both parts", "inbox");
+        email.body_plain = Some("a".repeat(100));
+        email.body_html = Some("b".repeat(250));
+        email.attachments = vec![nuncio_core::model::Attachment {
+            filename: "logo.png".to_string(),
+            mime_type: "image/png".to_string(),
+            content: vec![0u8; 650].into(),
+        }];
+
+        assert_eq!(CompiledFilter::message_size_bytes(&email), 1_000);
+        assert!(size_rule_matches(
+            "WHERE size = 1000 ACTION MARK READ",
+            &email
+        ));
+    }
+
+    /// A message with neither body part nor attachments genuinely measures
+    /// zero, and must still compare as a number rather than being skipped.
+    #[test]
+    fn size_of_empty_message_is_zero() {
+        let (mut email, _) = test_placed_email("acct-1", "Empty", "inbox");
+        email.body_plain = None;
+        email.body_html = None;
+
+        assert_eq!(CompiledFilter::message_size_bytes(&email), 0);
+        assert!(size_rule_matches("WHERE size < 1 ACTION MARK READ", &email));
+    }
+
     #[test]
     fn test_webhook_signature() {
         let sig =

crates/nuncio-store/src/db.rs

@@ -413,13 +413,36 @@ fn tls_mode_to_db(mode: nuncio_core::TlsMode) -> &'static str {
 }
 
 /// Parse a persisted `accounts` TLS-mode column back into
-/// [`nuncio_core::TlsMode`]. An unrecognized value falls back to the safe
-/// default (implicit TLS) rather than failing the whole account load.
-fn tls_mode_from_db(value: &str) -> nuncio_core::TlsMode {
+/// [`nuncio_core::TlsMode`].
+///
+/// Fails closed. TLS modes are validated before they are written, so the only
+/// way an unrecognized token reaches this function is corruption or tampering
+/// of the row. Coercing such a value to a default would silently hand the
+/// caller a transport the operator never configured -- reading `"start_tls"`
+/// as implicit TLS, or an unreadable value as *any* concrete mode, changes the
+/// account's security posture with no signal at all. The account's real mode is
+/// unknown at that point, and an unknown security posture is not something this
+/// layer may guess at, so the read is refused and the offending account, column,
+/// and raw value are named in the error.
+///
+/// This is deliberately stricter than the best-effort recovery path in
+/// [`crate::recovery`]: salvage exists to rescue what it can from an already
+/// broken file and has no caller to return an error to, whereas this is the
+/// path every ordinary account read takes.
+fn tls_mode_from_db(
+    account_id: &str,
+    column: &str,
+    value: &str,
+) -> Result<nuncio_core::TlsMode, DatabaseError> {
     match value {
-        "start_tls" => nuncio_core::TlsMode::StartTls,
-        "plain" => nuncio_core::TlsMode::Plain,
-        _ => nuncio_core::TlsMode::ImplicitTls,
+        "implicit_tls" => Ok(nuncio_core::TlsMode::ImplicitTls),
+        "start_tls" => Ok(nuncio_core::TlsMode::StartTls),
+        "plain" => Ok(nuncio_core::TlsMode::Plain),
+        other => Err(DatabaseError::Corrupted(format!(
+            "account {account_id} has an unrecognized {column} value {other:?}; refusing to \
+             load it under a guessed TLS mode -- re-save the account with a valid transport \
+             security setting"
+        ))),
     }
 }
 
@@ -1369,8 +1392,7 @@ impl DatabaseEngine {
         .await
         .map_err(DatabaseError::Query)?;
 
-        Ok(rows
-            .into_iter()
+        rows.into_iter()
             .map(
                 |(
                     id,
@@ -1387,7 +1409,8 @@ impl DatabaseEngine {
                     smtp_tls_mode,
                     collection_url,
                     filters_enabled,
-                )| {
+                )|
+                 -> Result<nuncio_core::AccountConfig, DatabaseError> {
                     let protocol = serde_json::from_str(&protocol_str)
                         .unwrap_or(nuncio_core::AccountProtocol::ImapSmtp);
                     // Reconstruct the exact transport variant from the
@@ -1410,10 +1433,18 @@ impl DatabaseEngine {
                             nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport {
                                 imap_host: server_host,
                                 imap_port: server_port as u16,
-                                imap_tls_mode: tls_mode_from_db(&imap_tls_mode),
+                                imap_tls_mode: tls_mode_from_db(
+                                    &id,
+                                    "imap_tls_mode",
+                                    &imap_tls_mode,
+                                )?,
                                 smtp_host: resolved_smtp_host,
                                 smtp_port: resolved_smtp_port,
-                                smtp_tls_mode: tls_mode_from_db(&smtp_tls_mode),
+                                smtp_tls_mode: tls_mode_from_db(
+                                    &id,
+                                    "smtp_tls_mode",
+                                    &smtp_tls_mode,
+                                )?,
                             })
                         }
                         nuncio_core::AccountProtocol::Jmap => {
@@ -1427,7 +1458,7 @@ impl DatabaseEngine {
                             })
                         }
                     };
-                    nuncio_core::AccountConfig {
+                    Ok(nuncio_core::AccountConfig {
                         id,
                         name,
                         email_address,
@@ -1435,10 +1466,10 @@ impl DatabaseEngine {
                         sync_interval_secs: sync_interval_secs as u64,
                         filters_enabled: filters_enabled != 0,
                         transport,
-                    }
+                    })
                 },
             )
-            .collect())
+            .collect()
     }
 
     /// Fetch the persisted inbound-sync checkpoint for a folder, or `None` if
@@ -4289,6 +4320,69 @@ mod tests {
         assert_eq!(fetched, acct);
     }
 
+    /// An unreadable stored TLS mode must fail the read rather than resolve to
+    /// a guessed one. The account below is configured for STARTTLS; its stored
+    /// mode is then overwritten out-of-band with a token this build does not
+    /// know. Previously that token was coerced to implicit TLS, so every
+    /// ordinary account read handed the caller a transport the operator never
+    /// chose, with nothing anywhere to say so. Both the IMAP and the SMTP
+    /// column are checked, and `get_account` is checked alongside
+    /// `list_accounts` because it reads through the same path.
+    #[tokio::test]
+    async fn unrecognized_stored_tls_mode_fails_the_read_instead_of_defaulting() {
+        for column in ["imap_tls_mode", "smtp_tls_mode"] {
+            let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap();
+
+            let acct = nuncio_core::AccountConfig {
+                id: "acct-tls-corrupt".to_string(),
+                name: "Tampered TLS Account".to_string(),
+                email_address: "tampered@nuncio.mx".to_string(),
+                keyring_secret_key: "nuncio/acct-tls-corrupt".to_string(),
+                sync_interval_secs: 60,
+                filters_enabled: false,
+                transport: nuncio_core::Transport::ImapSmtp(nuncio_core::ImapSmtpTransport {
+                    imap_host: "imap.nuncio.mx".to_string(),
+                    imap_port: 143,
+                    imap_tls_mode: nuncio_core::TlsMode::StartTls,
+                    smtp_host: "smtp.nuncio.mx".to_string(),
+                    smtp_port: 587,
+                    smtp_tls_mode: nuncio_core::TlsMode::StartTls,
+                }),
+            };
+            engine.save_account(&acct).await.expect("save succeeds");
+
+            // Simulates corruption or tampering of the row: writes are
+            // validated, so this value could never arrive through the API.
+            sqlx::query(&format!("UPDATE accounts SET {column} = ? WHERE id = ?"))
+                .bind("tls")
+                .bind("acct-tls-corrupt")
+                .execute(&engine.pool)
+                .await
+                .expect("corrupting update succeeds");
+
+            let err = engine
+                .list_accounts()
+                .await
+                .expect_err("an unreadable TLS mode must not resolve to a default");
+            let message = err.to_string();
+            assert!(
+                matches!(err, DatabaseError::Corrupted(_)),
+                "expected a corruption error, got: {message}"
+            );
+            for expected in ["acct-tls-corrupt", column, "tls"] {
+                assert!(
+                    message.contains(expected),
+                    "error must name {expected}, got: {message}"
+                );
+            }
+
+            engine
+                .get_account("acct-tls-corrupt")
+                .await
+                .expect_err("get_account must fail closed on the same row");
+        }
+    }
+
     /// A CalDAV account's `collection_url` must survive save/reload, and a
     /// mail account's empty `collection_url` must round-trip as empty (not
     /// NULL-mangled), proving the backfill-safe column persists both.