fix(store): deterministic message surrogate identity (C1) #337

Merged
KofTwentyTwo merged 2 commits from story/ws-b1-message-identity into dev 2026-07-31 18:57:22 +00:00
18 changed files with 510 additions and 92 deletions

No files matched your search

+4
View File
@@ -185,6 +185,8 @@ mod tests {
id: "msg-100".to_string(),
account_id: "acct-1".to_string(),
folder_id: "INBOX".to_string(),
remote_id: "100".to_string(),
uid_validity: "1".to_string(),
subject: "Test Subject 1".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -198,6 +200,8 @@ mod tests {
id: "msg-101".to_string(),
account_id: "acct-1".to_string(),
folder_id: "INBOX".to_string(),
remote_id: "101".to_string(),
uid_validity: "1".to_string(),
subject: "Test Subject 2".to_string(),
sender: "charlie@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
+80 -1
View File
@@ -2,6 +2,7 @@
use bytes::Bytes;
use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
/// Real-time daemon health, performance metrics, and telemetry payload.
#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -63,12 +64,30 @@ pub struct Attachment {
/// Email message domain entity owned by Nuncio core.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Email {
/// Unique message identifier.
/// Opaque, deterministic surrogate identifier for the message.
///
/// Derived from [`Email::surrogate_id`] over the account, folder,
/// UIDVALIDITY, and protocol-native id. It carries no decodable internal
/// encoding (callers must treat it as opaque) and is stable across
/// re-syncs of the same message, so persisting it upserts the same row
/// rather than duplicating. Distinct messages that happen to share a
/// protocol id across folders or accounts hash to distinct surrogates, so
/// one can never silently overwrite another.
pub id: String,
/// Account identifier owning the message.
pub account_id: String,
/// Mailbox folder identifier (e.g. "inbox").
pub folder_id: String,
/// Protocol-native message id used to address the message on its server:
/// the IMAP UID as a decimal string, or the JMAP Email object id. This is
/// what a remote mutation must use to identify the message on the wire --
/// never the opaque [`Email::id`].
pub remote_id: String,
/// The addressing scope the `remote_id` is only meaningful within: the IMAP
/// folder UIDVALIDITY as a decimal string. Protocols without a UIDVALIDITY
/// (JMAP) carry a stable sentinel instead, so the surrogate id stays
/// deterministic.
pub uid_validity: String,
/// Subject line.
pub subject: String,
/// Sender address (e.g. "alice@nuncio.mx").
@@ -87,6 +106,33 @@ pub struct Email {
pub attachments: Vec<Attachment>,
}
impl Email {
/// Compute the deterministic, collision-resistant surrogate id for a
/// message from its addressing coordinates.
///
/// The four coordinates together uniquely identify a stored message: a
/// protocol-native `remote_id` (IMAP UID / JMAP object id) is only unique
/// within one `uid_validity` scope of one `folder_id` of one `account_id`,
/// so all four must participate. Each field is fed length-prefixed so that
/// no two distinct coordinate tuples can serialize to the same byte stream
/// (e.g. `("a", "bc")` and `("ab", "c")` must not collide). The SHA-256
/// digest is hex-encoded, yielding a stable opaque token that leaks none of
/// its inputs' encoding on the wire.
pub fn surrogate_id(
account_id: &str,
folder_id: &str,
uid_validity: &str,
remote_id: &str,
) -> String {
let mut hasher = Sha256::new();
for field in [account_id, folder_id, uid_validity, remote_id] {
hasher.update((field.len() as u64).to_le_bytes());
hasher.update(field.as_bytes());
}
hex::encode(hasher.finalize())
}
}
/// Mailbox folder entity.
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct Folder {
@@ -168,4 +214,37 @@ mod tests {
assert_eq!(parsed.status, "HEALTHY");
assert_eq!(parsed.total_emails, 15000);
}
#[test]
fn surrogate_id_is_deterministic_and_opaque() {
let a = Email::surrogate_id("acct-1", "INBOX", "42", "5");
let b = Email::surrogate_id("acct-1", "INBOX", "42", "5");
assert_eq!(a, b, "identical coordinates must hash identically");
// A 32-byte SHA-256 digest hex-encodes to 64 chars, and the token must
// not leak the protocol-native id it was built from.
assert_eq!(a.len(), 64);
assert!(a.chars().all(|c| c.is_ascii_hexdigit()));
assert!(!a.contains("imap-uid-"));
}
#[test]
fn surrogate_id_distinguishes_the_cross_folder_and_cross_account_collision() {
// The same protocol UID (5) in two different folders of one account, or
// in two different accounts, must NOT collapse to the same surrogate.
let inbox = Email::surrogate_id("acct-1", "INBOX", "42", "5");
let sent = Email::surrogate_id("acct-1", "Sent", "7", "5");
let other_account = Email::surrogate_id("acct-2", "INBOX", "42", "5");
assert_ne!(inbox, sent, "same UID in two folders must not collide");
assert_ne!(
inbox, other_account,
"same UID in two accounts must not collide"
);
// Field boundaries are unambiguous: shifting a delimiter-like split
// across two adjacent fields yields a different id.
assert_ne!(
Email::surrogate_id("a", "bc", "1", "1"),
Email::surrogate_id("ab", "c", "1", "1"),
);
}
}
+4
View File
@@ -373,6 +373,8 @@ mod tests {
id: "msg-1".to_string(),
account_id: account_id.to_string(),
folder_id: folder_id.to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -394,6 +396,8 @@ mod tests {
id: "msg-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: "Urgent Meeting".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
+19 -13
View File
@@ -36,25 +36,31 @@ pub enum RemoteMutationKind {
Delete,
}
/// A fully-addressed remote mutation for a single message: which message
/// (protocol-native id), the folder it currently lives in, that folder's
/// stored sync checkpoint (carrying IMAP UIDVALIDITY), and the change to
/// apply.
/// A fully-addressed remote mutation for a single message: the protocol-native
/// id to address it on the server, the folder it currently lives in, the
/// UIDVALIDITY scope that id was captured under, and the change to apply.
///
/// `folder_checkpoint` is the value persisted by a prior sync
/// (`"{uidvalidity}:{uidnext}"` for IMAP). The IMAP backend uses it to refuse
/// to act when the mailbox's current UIDVALIDITY no longer matches -- a
/// renumbered mailbox reassigns UIDs, so the stored UID would otherwise
/// address the wrong message. Protocols that do not use UIDVALIDITY (JMAP)
/// ignore it.
/// Addressing is driven by `remote_id` (the IMAP UID / JMAP object id), NOT by
/// the opaque `message_id` -- so remote-mutation targeting never depends on the
/// shape of the surrogate id. `uid_validity` is the mailbox UIDVALIDITY the
/// `remote_id` was captured under; the IMAP backend refuses to act unless the
/// mailbox's current UIDVALIDITY still matches it, because a renumbered mailbox
/// (RFC 3501 s2.3.1.1) reassigns UIDs and the stored UID would otherwise
/// address the wrong message. Protocols without a UIDVALIDITY (JMAP) carry a
/// stable sentinel and ignore this guard.
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct RemoteMutationSpec {
/// Protocol-native message id (IMAP `imap-uid-{uid}`, JMAP object id).
/// Opaque surrogate id of the target message. Diagnostic only -- it names
/// the message in errors/logs and is never used to address it on the wire.
pub message_id: String,
/// Protocol-native id used to address the message on its server: the IMAP
/// UID as a decimal string, or the JMAP Email object id.
pub remote_id: String,
/// Folder the message currently lives in.
pub folder_id: String,
/// The source folder's stored sync checkpoint, if any.
pub folder_checkpoint: Option<String>,
/// The UIDVALIDITY the `remote_id` was captured under (IMAP; decimal
/// string), or a protocol sentinel for servers without UIDVALIDITY (JMAP).
pub uid_validity: String,
/// The change to apply.
pub kind: RemoteMutationKind,
}
+54 -42
View File
@@ -34,31 +34,21 @@ const FETCH_ITEM_TIMEOUT: Duration = Duration::from_secs(60);
use crate::backend::{MailBackend, RemoteMutationKind, RemoteMutationSpec};
use crate::parser::{MailError, MimeParserAdapter};
/// Prefix an IMAP [`Email::id`] carries in front of its raw numeric UID (see
/// [`ImapEngine::sync_folder_messages_with_session`], which formats every id as
/// `"imap-uid-{uid}"`). A remote mutation must recover the UID from the id to
/// address the message, so the prefix is defined once here.
const IMAP_UID_ID_PREFIX: &str = "imap-uid-";
/// Recover the raw IMAP UID from an [`Email::id`] of the form
/// `"imap-uid-{uid}"`. Returns `None` for any id that does not carry the
/// expected prefix and a valid non-zero `u32` UID, so a mutation against a
/// malformed/foreign id fails honestly rather than addressing UID 0.
fn parse_imap_uid(message_id: &str) -> Option<u32> {
let raw = message_id.strip_prefix(IMAP_UID_ID_PREFIX)?;
let uid = raw.trim().parse::<u32>().ok()?;
/// Recover a raw IMAP UID from a [`RemoteMutationSpec::remote_id`] (a decimal
/// string). Returns `None` for anything that is not a valid non-zero `u32`, so
/// a mutation against a malformed/foreign remote id fails honestly rather than
/// addressing UID 0.
fn parse_imap_uid(remote_id: &str) -> Option<u32> {
let uid = remote_id.trim().parse::<u32>().ok()?;
(uid >= 1).then_some(uid)
}
/// Extract the UIDVALIDITY component of a stored folder checkpoint
/// (`"{uidvalidity}:{uidnext}"`). Returns `None` for a missing, malformed, or
/// legacy bare-number checkpoint -- the caller treats an absent stored
/// UIDVALIDITY as "cannot prove the mailbox has not been renumbered" and
/// refuses to act.
fn checkpoint_uid_validity(checkpoint: Option<&str>) -> Option<u32> {
let raw = checkpoint?;
let (validity, _uid) = raw.split_once(CHECKPOINT_DELIM)?;
validity.trim().parse::<u32>().ok()
/// Parse a stored [`RemoteMutationSpec::uid_validity`] (a decimal string) into
/// a `u32`. Returns `None` for a missing or non-numeric value -- the caller
/// treats an unparseable UIDVALIDITY as "cannot prove the mailbox has not been
/// renumbered" and refuses to act.
fn parse_uid_validity(uid_validity: &str) -> Option<u32> {
uid_validity.trim().parse::<u32>().ok()
}
/// Delimiter between the two components of a folder sync checkpoint. The
@@ -652,7 +642,16 @@ impl ImapEngine {
let uid_num = fetch_data.uid.unwrap_or(0);
max_uid_seen = max_uid_seen.max(uid_num);
let email_id = format!("imap-uid-{}", uid_num);
// The message is addressed on the wire by its UID within the
// folder's UIDVALIDITY scope; the persisted id is an opaque
// surrogate hashed over both plus the account and folder, so the
// same UID in another folder/account can never collide.
let remote_id = uid_num.to_string();
let uid_validity = server_uid_validity
.map(|v| v.to_string())
.unwrap_or_default();
let email_id =
Email::surrogate_id(&self.account_id, folder_id, &uid_validity, &remote_id);
let is_read = fetch_data
.flags()
@@ -665,6 +664,8 @@ impl ImapEngine {
folder_id,
raw_bytes,
)?;
email.remote_id = remote_id;
email.uid_validity = uid_validity;
email.read = is_read;
email
} else {
@@ -721,6 +722,8 @@ impl ImapEngine {
id: email_id,
account_id: self.account_id.clone(),
folder_id: folder_id.to_string(),
remote_id,
uid_validity,
subject,
sender,
recipient,
@@ -815,10 +818,10 @@ impl ImapEngine {
where
S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static,
{
let uid = parse_imap_uid(&spec.message_id).ok_or_else(|| {
let uid = parse_imap_uid(&spec.remote_id).ok_or_else(|| {
MailError::ImapError(format!(
"cannot parse an IMAP UID from message id '{}'",
spec.message_id
"cannot parse an IMAP UID from remote id '{}' (message '{}')",
spec.remote_id, spec.message_id
))
})?;
@@ -831,7 +834,7 @@ impl ImapEngine {
// Refuse to act unless the mailbox's current UIDVALIDITY provably
// matches the one the stored UID was captured under.
let stored_validity = checkpoint_uid_validity(spec.folder_checkpoint.as_deref());
let stored_validity = parse_uid_validity(&spec.uid_validity);
match (stored_validity, mailbox.uid_validity) {
(Some(stored), Some(current)) if stored == current => {}
(stored, current) => {
@@ -1575,23 +1578,24 @@ mod tests {
}
#[test]
fn parse_imap_uid_recovers_uid_and_rejects_malformed_ids() {
assert_eq!(parse_imap_uid("imap-uid-42"), Some(42));
assert_eq!(parse_imap_uid("imap-uid-1"), Some(1));
// UID 0 is not a valid IMAP UID; a bare/foreign id yields nothing.
assert_eq!(parse_imap_uid("imap-uid-0"), None);
assert_eq!(parse_imap_uid("42"), None);
fn parse_imap_uid_recovers_uid_and_rejects_non_numeric_ids() {
assert_eq!(parse_imap_uid("42"), Some(42));
assert_eq!(parse_imap_uid(" 42 "), Some(42));
assert_eq!(parse_imap_uid("1"), Some(1));
// UID 0 is not a valid IMAP UID; a non-numeric/foreign id yields nothing.
assert_eq!(parse_imap_uid("0"), None);
assert_eq!(parse_imap_uid("imap-uid-42"), None);
assert_eq!(parse_imap_uid("jmap-object-id"), None);
}
#[test]
fn checkpoint_uid_validity_extracts_only_a_well_formed_validity() {
assert_eq!(checkpoint_uid_validity(Some("7:105")), Some(7));
assert_eq!(checkpoint_uid_validity(Some(" 7 : 105 ")), Some(7));
assert_eq!(checkpoint_uid_validity(None), None);
// A legacy bare-number checkpoint carries no validity to compare.
assert_eq!(checkpoint_uid_validity(Some("105")), None);
assert_eq!(checkpoint_uid_validity(Some("x:105")), None);
fn parse_uid_validity_accepts_only_a_decimal_value() {
assert_eq!(parse_uid_validity("7"), Some(7));
assert_eq!(parse_uid_validity(" 7 "), Some(7));
assert_eq!(parse_uid_validity(""), None);
// The JMAP sentinel and a full checkpoint string are not a bare validity.
assert_eq!(parse_uid_validity("jmap"), None);
assert_eq!(parse_uid_validity("7:105"), None);
}
/// Drive a mutation over a scripted IMAP server (in-memory duplex),
@@ -1697,10 +1701,18 @@ mod tests {
}
fn imap_spec(kind: RemoteMutationKind, checkpoint: Option<&str>) -> RemoteMutationSpec {
// The tests express the captured scope as a "{uidvalidity}:{uidnext}"
// checkpoint string (mirroring how a sync stores it); the spec now
// carries just the UIDVALIDITY component the guard compares against.
let uid_validity = checkpoint
.and_then(|c| c.split(':').next())
.unwrap_or_default()
.to_string();
RemoteMutationSpec {
message_id: "imap-uid-42".to_string(),
message_id: "surrogate-42".to_string(),
remote_id: "42".to_string(),
folder_id: "INBOX".to_string(),
folder_checkpoint: checkpoint.map(str::to_string),
uid_validity,
kind,
}
}
+30 -6
View File
@@ -8,6 +8,14 @@ use serde_json::{json, Value};
use crate::backend::{MailBackend, RemoteMutationKind, RemoteMutationSpec};
use crate::parser::MailError;
/// Stand-in for the UIDVALIDITY scope on JMAP, which has no such concept: a
/// JMAP Email object id is already globally stable within its account (RFC
/// 8621), so a fixed sentinel keeps the surrogate-id computation uniform with
/// IMAP without implying a UIDVALIDITY guard. The IMAP UIDVALIDITY guard treats
/// this non-numeric value as "no validity to compare" and is never engaged for
/// a JMAP mutation, which addresses messages by object id.
const JMAP_UID_VALIDITY_SENTINEL: &str = "jmap";
/// JMAP Session Object (RFC 8620 Section 2).
#[derive(Debug, Clone, Serialize, Deserialize)]
#[serde(rename_all = "camelCase")]
@@ -214,7 +222,9 @@ impl JmapEngine {
/// `$flagged` keyword (`true` to set, `null` to clear), the JMAP equivalent
/// of IMAP `\Flagged`.
pub fn build_email_set_request(account_id: &str, spec: &RemoteMutationSpec) -> Value {
let id = &spec.message_id;
// Address the message by its protocol-native JMAP object id, never the
// opaque surrogate `message_id`.
let id = &spec.remote_id;
let (update, destroy) = match &spec.kind {
RemoteMutationKind::SetFlagged { value } => {
let keyword = if *value { json!(true) } else { Value::Null };
@@ -383,10 +393,23 @@ impl JmapEngine {
.map(|a| a.email.clone())
.unwrap_or_else(|| "me@nuncio.mx".to_string());
let folder_id = "inbox".to_string();
// The JMAP object id is the protocol-native id; the persisted
// id is an opaque surrogate hashed over it plus the account,
// folder, and (sentinel) scope so it stays uniform with IMAP.
let remote_id = item.id;
let id = Email::surrogate_id(
&self.account_id,
&folder_id,
JMAP_UID_VALIDITY_SENTINEL,
&remote_id,
);
Email {
id: item.id,
id,
account_id: self.account_id.clone(),
folder_id: "inbox".to_string(),
folder_id,
remote_id,
uid_validity: JMAP_UID_VALIDITY_SENTINEL.to_string(),
subject: item.subject.unwrap_or_else(|| "No Subject".to_string()),
sender,
recipient,
@@ -558,7 +581,7 @@ impl MailBackend for JmapEngine {
let request = Self::build_email_set_request(&account_id, spec);
let raw = self.post_jmap(&session.api_url, &request).await?;
let expect_destroy = matches!(spec.kind, RemoteMutationKind::Delete);
Self::confirm_email_set_applied(&raw, &spec.message_id, expect_destroy)
Self::confirm_email_set_applied(&raw, &spec.remote_id, expect_destroy)
}
}
@@ -689,9 +712,10 @@ mod tests {
fn spec(kind: RemoteMutationKind) -> RemoteMutationSpec {
RemoteMutationSpec {
message_id: "m-1".to_string(),
message_id: "surrogate-m-1".to_string(),
remote_id: "m-1".to_string(),
folder_id: "mb-inbox".to_string(),
folder_checkpoint: None,
uid_validity: JMAP_UID_VALIDITY_SENTINEL.to_string(),
kind,
}
}
+2
View File
@@ -244,6 +244,8 @@ mod tests {
id: "msg-mock-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: "Mock Test".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
+4
View File
@@ -129,6 +129,10 @@ impl MimeParserAdapter {
id: id.to_string(),
account_id: account_id.to_string(),
folder_id: folder_id.to_string(),
// The MIME body carries no protocol addressing; the caller (the
// IMAP sync) fills these from the transport before persisting.
remote_id: String::new(),
uid_validity: String::new(),
subject,
sender,
recipient,
+2
View File
@@ -329,6 +329,8 @@ mod tests {
id: "msg-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "outbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: "Status Update".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
+11 -4
View File
@@ -84,7 +84,11 @@ async fn wiremock_jmap_session_discovery_and_email_get_sync() {
.expect("parse email get");
assert_eq!(emails.len(), 1);
assert_eq!(emails[0].id, "msg-wm-100");
// The JMAP object id is preserved as the protocol-native `remote_id`, while
// the persisted id is an opaque 64-char surrogate hash (never the raw id).
assert_eq!(emails[0].remote_id, "msg-wm-100");
assert_ne!(emails[0].id, "msg-wm-100");
assert_eq!(emails[0].id.len(), 64);
assert_eq!(emails[0].subject, "WireMock JMAP E2E Test");
assert_eq!(emails[0].sender, "sender@nuncio.mx");
assert_eq!(new_state, "sync-state-500");
@@ -200,7 +204,9 @@ async fn jmap_engine_sync_folders_and_messages_through_mail_backend_trait() {
.await
.expect("sync_messages succeeds");
assert_eq!(emails.len(), 1);
assert_eq!(emails[0].id, "msg-wm-200");
assert_eq!(emails[0].remote_id, "msg-wm-200");
assert_ne!(emails[0].id, "msg-wm-200");
assert_eq!(emails[0].id.len(), 64);
assert_eq!(emails[0].subject, "Trait-Level JMAP Sync");
assert_eq!(emails[0].sender, "sender@nuncio.mx");
assert_eq!(emails[0].received_at, 1700002000);
@@ -264,9 +270,10 @@ fn jmap_engine(mock_server: &MockServer) -> JmapEngine {
fn spec(kind: RemoteMutationKind) -> RemoteMutationSpec {
RemoteMutationSpec {
message_id: "jmap-msg-1".to_string(),
message_id: "surrogate-jmap-msg-1".to_string(),
remote_id: "jmap-msg-1".to_string(),
folder_id: "mb-inbox".to_string(),
folder_checkpoint: None,
uid_validity: "jmap".to_string(),
kind,
}
}
+251 -6
View File
@@ -471,6 +471,8 @@ impl DatabaseEngine {
id TEXT PRIMARY KEY NOT NULL,
account_id TEXT NOT NULL,
folder_id TEXT NOT NULL,
remote_id TEXT NOT NULL DEFAULT '',
uid_validity TEXT NOT NULL DEFAULT '',
subject TEXT NOT NULL,
sender TEXT NOT NULL,
recipient TEXT NOT NULL,
@@ -711,11 +713,84 @@ impl DatabaseEngine {
self.ensure_accounts_smtp_columns().await?;
self.ensure_accounts_tls_mode_columns().await?;
self.ensure_accounts_dav_columns().await?;
self.ensure_messages_identity_columns().await?;
self.backfill_message_fts().await?;
Ok(())
}
/// Additive, backfill-safe migration that adds the `remote_id` /
/// `uid_validity` message-identity columns to a pre-existing `messages`
/// table that predated the opaque surrogate id, and installs the
/// `UNIQUE(account_id, folder_id, uid_validity, remote_id)` index that is
/// the real correctness guard against a shared protocol id (e.g. an IMAP
/// UID reused across folders/accounts) overwriting an unrelated message.
///
/// A fresh database already gets both columns from `CREATE TABLE IF NOT
/// EXISTS messages` above, so on a fresh database the `PRAGMA table_info`
/// check finds them present and the backfill matches no rows -- only the
/// index is created. For a pre-existing database file the columns are added
/// with an empty-string default and every legacy row is backfilled to
/// `remote_id = id` (the old primary key, which was globally unique) so the
/// UNIQUE index can be built without collisions. SQLite has no `ADD COLUMN
/// IF NOT EXISTS`, so column presence is checked explicitly first, making
/// this safe to run on every daemon startup.
///
/// The whole step runs in a single transaction, and the backfill is
/// unconditional (idempotent `WHERE remote_id = ''`) rather than gated on
/// having just added the column. Both properties make a partial run
/// self-healing: a crash mid-migration rolls back atomically, and even a
/// committed intermediate state (column present but a legacy row still
/// carrying the empty default) is completed by the next startup's backfill
/// before the index is built -- so the daemon can never wedge on an index
/// collision it could have avoided.
async fn ensure_messages_identity_columns(&self) -> Result<(), DatabaseError> {
let existing_columns: Vec<String> = sqlx::query("PRAGMA table_info(messages)")
.fetch_all(&self.pool)
.await
.map_err(DatabaseError::Query)?
.iter()
.map(|row| row.get::<String, _>("name"))
.collect();
let mut tx = self.pool.begin().await.map_err(DatabaseError::Query)?;
if !existing_columns.iter().any(|c| c == "remote_id") {
sqlx::query("ALTER TABLE messages ADD COLUMN remote_id TEXT NOT NULL DEFAULT ''")
.execute(&mut *tx)
.await
.map_err(DatabaseError::Query)?;
}
if !existing_columns.iter().any(|c| c == "uid_validity") {
sqlx::query("ALTER TABLE messages ADD COLUMN uid_validity TEXT NOT NULL DEFAULT ''")
.execute(&mut *tx)
.await
.map_err(DatabaseError::Query)?;
}
// Seed any legacy row still carrying the empty-string default with a
// unique, non-empty remote id (the old primary key) so the UNIQUE
// identity index is satisfiable. Unconditional so a partial prior run
// is always completed before the index is (re)built; a no-op on a fresh
// or already-migrated database.
sqlx::query("UPDATE messages SET remote_id = id WHERE remote_id = ''")
.execute(&mut *tx)
.await
.map_err(DatabaseError::Query)?;
sqlx::query(
"CREATE UNIQUE INDEX IF NOT EXISTS idx_messages_identity \
ON messages (account_id, folder_id, uid_validity, remote_id)",
)
.execute(&mut *tx)
.await
.map_err(DatabaseError::Query)?;
tx.commit().await.map_err(DatabaseError::Query)?;
Ok(())
}
/// Additive, backfill-safe migration that adds the `imap_tls_mode` /
/// `smtp_tls_mode` columns to a pre-existing `accounts` table that
/// predates persisted per-protocol TLS transport modes.
@@ -1081,13 +1156,15 @@ impl DatabaseEngine {
sqlx::query(
r#"
INSERT OR REPLACE INTO messages
(id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
(id, account_id, folder_id, remote_id, uid_validity, subject, sender, recipient, received_at, read_flag, body_plain, body_html)
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
"#,
)
.bind(&email.id)
.bind(&email.account_id)
.bind(&email.folder_id)
.bind(&email.remote_id)
.bind(&email.uid_validity)
.bind(&email.subject)
.bind(&email.sender)
.bind(&email.recipient)
@@ -1143,9 +1220,11 @@ impl DatabaseEngine {
i64,
Option<String>,
Option<String>,
String,
String,
)> = sqlx::query_as(
r#"
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity
FROM messages
WHERE folder_id = ?
ORDER BY received_at DESC
@@ -1171,6 +1250,8 @@ impl DatabaseEngine {
read_flag,
body_plain,
body_html,
remote_id,
uid_validity,
)| {
let dec_plain = body_plain
.map(|p| {
@@ -1194,6 +1275,8 @@ impl DatabaseEngine {
id,
account_id,
folder_id,
remote_id,
uid_validity,
subject,
sender,
recipient,
@@ -1225,9 +1308,11 @@ impl DatabaseEngine {
i64,
Option<String>,
Option<String>,
String,
String,
) = sqlx::query_as(
r#"
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html
SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity
FROM messages
WHERE id = ?
"#,
@@ -1252,6 +1337,8 @@ impl DatabaseEngine {
id: row.0,
account_id: row.1,
folder_id: row.2,
remote_id: row.10,
uid_validity: row.11,
subject: row.3,
sender: row.4,
recipient: row.5,
@@ -1902,7 +1989,7 @@ impl DatabaseEngine {
limit: usize,
) -> Result<Vec<nuncio_core::model::Email>, DatabaseError> {
let mut builder: sqlx::QueryBuilder<'_, sqlx::Sqlite> = sqlx::QueryBuilder::new(
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html FROM messages "
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity FROM messages "
);
if !last_id.is_empty() {
builder.push("WHERE id > ");
@@ -1922,6 +2009,8 @@ impl DatabaseEngine {
i64,
Option<String>,
Option<String>,
String,
String,
)>();
let rows = query
@@ -1947,6 +2036,8 @@ impl DatabaseEngine {
id: r.0,
account_id: r.1,
folder_id: r.2,
remote_id: r.10,
uid_validity: r.11,
subject: r.3,
sender: r.4,
recipient: r.5,
@@ -1972,7 +2063,7 @@ impl DatabaseEngine {
folder_id: Option<&str>,
) -> Result<Vec<nuncio_core::model::Email>, DatabaseError> {
let mut builder: sqlx::QueryBuilder<'_, sqlx::Sqlite> = sqlx::QueryBuilder::new(
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html FROM messages"
"SELECT id, account_id, folder_id, subject, sender, recipient, received_at, read_flag, body_plain, body_html, remote_id, uid_validity FROM messages"
);
let mut has_filter = false;
@@ -2002,6 +2093,8 @@ impl DatabaseEngine {
i64,
Option<String>,
Option<String>,
String,
String,
)>();
let rows = query
@@ -2027,6 +2120,8 @@ impl DatabaseEngine {
id: r.0,
account_id: r.1,
folder_id: r.2,
remote_id: r.10,
uid_validity: r.11,
subject: r.3,
sender: r.4,
recipient: r.5,
@@ -2483,6 +2578,8 @@ mod tests {
id: "msg-db-100".to_string(),
account_id: "acct-1".to_string(),
folder_id: "INBOX".to_string(),
remote_id: "100".to_string(),
uid_validity: "1".to_string(),
subject: "Database Sync Test".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -2530,6 +2627,8 @@ mod tests {
id: "msg-db-corrupt".to_string(),
account_id: "acct-1".to_string(),
folder_id: "INBOX".to_string(),
remote_id: "corrupt".to_string(),
uid_validity: "1".to_string(),
subject: "Corrupted At Rest".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -2832,6 +2931,8 @@ mod tests {
id: "msg-mark-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "INBOX".to_string(),
remote_id: "mark-1".to_string(),
uid_validity: "1".to_string(),
subject: "Mark Read Test".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -3362,6 +3463,8 @@ mod tests {
id: format!("msg-{i:03}"),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: format!("{i}"),
uid_validity: "1".to_string(),
subject: format!("Subject {i}"),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -3469,11 +3572,153 @@ mod tests {
);
}
/// Build a message exactly as a real sync would: an opaque surrogate id
/// hashed over its addressing coordinates, with the protocol-native id and
/// UIDVALIDITY scope carried in their own columns.
fn synced_email(
account_id: &str,
folder_id: &str,
uid_validity: &str,
remote_id: &str,
) -> nuncio_core::model::Email {
nuncio_core::model::Email {
id: nuncio_core::model::Email::surrogate_id(
account_id,
folder_id,
uid_validity,
remote_id,
),
account_id: account_id.to_string(),
folder_id: folder_id.to_string(),
remote_id: remote_id.to_string(),
uid_validity: uid_validity.to_string(),
subject: format!("{folder_id}/{remote_id}"),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
received_at: 1_700_000_000,
read: false,
body_plain: Some("body".to_string()),
body_html: None,
attachments: Vec::new(),
}
}
/// The C1 regression: the same protocol UID reused across folders or
/// accounts must persist as DISTINCT rows (never silently overwrite), and a
/// re-sync of the SAME message must upsert the one row it already owns.
#[tokio::test]
async fn message_identity_prevents_cross_folder_and_cross_account_collision() {
let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap();
// Same UID (5) in two folders of one account, and again in a second
// account -- the exact shapes that used to collapse to `imap-uid-5`.
let inbox = synced_email("acct-1", "INBOX", "42", "5");
let sent = synced_email("acct-1", "Sent", "7", "5");
let other_account = synced_email("acct-2", "INBOX", "99", "5");
// Every surrogate id is distinct, so none can overwrite another.
assert_ne!(inbox.id, sent.id);
assert_ne!(inbox.id, other_account.id);
assert_ne!(sent.id, other_account.id);
engine.save_email(&inbox).await.expect("save inbox");
engine.save_email(&sent).await.expect("save sent");
engine
.save_email(&other_account)
.await
.expect("save other account");
// All three coexist as separate rows, addressed by their own folder.
assert_eq!(engine.list_messages("INBOX", 10).await.unwrap().len(), 2);
assert_eq!(engine.list_messages("Sent", 10).await.unwrap().len(), 1);
// Re-syncing the SAME message recomputes the SAME id, so the upsert
// updates the one row rather than duplicating it.
let inbox_resynced = synced_email("acct-1", "INBOX", "42", "5");
assert_eq!(inbox_resynced.id, inbox.id);
engine
.save_email(&inbox_resynced)
.await
.expect("re-sync upserts");
assert_eq!(
engine.list_messages("INBOX", 10).await.unwrap().len(),
2,
"a re-sync must upsert, not duplicate"
);
// The recovered row round-trips its protocol addressing columns.
let fetched = engine.get_message(&inbox.id).await.expect("get message");
assert_eq!(fetched.remote_id, "5");
assert_eq!(fetched.uid_validity, "42");
assert_eq!(fetched.folder_id, "INBOX");
}
/// A crash between `ADD COLUMN remote_id` and its backfill can commit an
/// intermediate state -- the column present but a legacy row still carrying
/// the empty-string default -- with the UNIQUE index not yet built. The
/// migration must complete that backfill on the next open and build the
/// index without a collision, rather than wedging the daemon.
#[tokio::test]
async fn messages_identity_migration_completes_from_a_partial_run() {
let (engine, _dir) = DatabaseEngine::connect_ephemeral().await.unwrap();
// Reproduce the partial state: drop the identity index, then insert two
// rows in the same folder that both still hold the empty default. With
// the index gone this is permitted, and the two would collide on
// (account, folder, uid_validity='', remote_id='') if it were rebuilt now.
sqlx::query("DROP INDEX IF EXISTS idx_messages_identity")
.execute(engine.pool())
.await
.unwrap();
for id in ["legacy-a", "legacy-b"] {
sqlx::query(
"INSERT INTO messages \
(id, account_id, folder_id, remote_id, uid_validity, subject, sender, recipient, received_at, read_flag) \
VALUES (?, 'acct-1', 'INBOX', '', '', 'subj', 'a@nuncio.mx', 'b@nuncio.mx', 0, 0)",
)
.bind(id)
.execute(engine.pool())
.await
.unwrap();
}
// Re-running the migration must finish the backfill and build the index.
engine
.ensure_messages_identity_columns()
.await
.expect("a partial migration must complete on the next open");
// Both legacy rows survive with a unique, non-empty backfilled remote id.
assert_eq!(
engine.get_message("legacy-a").await.unwrap().remote_id,
"legacy-a"
);
assert_eq!(
engine.get_message("legacy-b").await.unwrap().remote_id,
"legacy-b"
);
// The unique identity index is now present, and re-running is idempotent.
let index: Option<(String,)> = sqlx::query_as(
"SELECT name FROM sqlite_master WHERE type = 'index' AND name = 'idx_messages_identity'",
)
.fetch_optional(engine.pool())
.await
.unwrap();
assert!(index.is_some(), "the unique identity index must be built");
engine
.ensure_messages_identity_columns()
.await
.expect("re-running an already-complete migration is a no-op");
}
fn export_test_email(id: &str, account_id: &str, folder_id: &str) -> nuncio_core::model::Email {
nuncio_core::model::Email {
id: id.to_string(),
account_id: account_id.to_string(),
folder_id: folder_id.to_string(),
remote_id: id.to_string(),
uid_validity: "1".to_string(),
subject: format!("Subject {id}"),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
+2
View File
@@ -105,6 +105,8 @@ mod tests {
id: id.to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: id.to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
+15
View File
@@ -1592,6 +1592,9 @@ fn synthetic_preview_email(id: &str) -> nuncio_core::model::Email {
id: id.to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
// Synthetic preview only -- never persisted or mutated remotely.
remote_id: id.to_string(),
uid_validity: String::new(),
subject: "Test Subject".to_string(),
sender: "test@nuncio.mx".to_string(),
recipient: "me@nuncio.mx".to_string(),
@@ -3434,6 +3437,8 @@ mod tests {
id: id.to_string(),
account_id: "acct-mail-1".to_string(),
folder_id: folder_id.to_string(),
remote_id: id.to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -4320,6 +4325,8 @@ mod tests {
id: "msg-filters-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -4420,6 +4427,8 @@ mod tests {
id: "msg-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: "anything".to_string(),
sender: "a@b.com".to_string(),
recipient: "c@d.com".to_string(),
@@ -4562,6 +4571,8 @@ mod tests {
id: "msg-preview-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "preview-1".to_string(),
uid_validity: "1".to_string(),
subject: "Urgent Board Meeting".to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -4728,6 +4739,8 @@ mod tests {
id: "msg-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: "Urgent Meeting".to_string(),
sender: "a@b.com".to_string(),
recipient: "c@d.com".to_string(),
@@ -4882,6 +4895,8 @@ mod tests {
id: "msg-1".to_string(),
account_id: "acct-1".to_string(),
folder_id: "inbox".to_string(),
remote_id: "1".to_string(),
uid_validity: "1".to_string(),
subject: "Urgent Meeting".to_string(),
sender: "a@b.com".to_string(),
recipient: "c@d.com".to_string(),
+10 -13
View File
@@ -204,7 +204,7 @@ async fn execute_one(
match mutation.mutation_type.as_str() {
"MOVE" | "COPY" | "FLAG" | "UNFLAG" | "DELETE" => {
dispatch_mailbox_mutation(db, env, mutation, &email, &payload).await
dispatch_mailbox_mutation(env, mutation, &email, &payload).await
}
"FORWARD" => dispatch_forward(db, env, &email, &payload).await,
"WEBHOOK" => dispatch_webhook(env, mutation, &email, &payload).await,
@@ -213,10 +213,10 @@ async fn execute_one(
}
/// Execute a mailbox-affecting mutation (move/copy/flag/unflag/delete) against
/// the account's mail backend, threading the source folder's stored checkpoint
/// so the backend can enforce its UIDVALIDITY guard.
/// the account's mail backend, recovering the remote addressing (protocol id +
/// UIDVALIDITY scope) from the stored message row so the backend can enforce
/// its UIDVALIDITY guard.
async fn dispatch_mailbox_mutation(
db: &DatabaseEngine,
env: &dyn RemoteExecutionEnv,
mutation: &PendingRemoteMutation,
email: &nuncio_core::model::Email,
@@ -245,23 +245,20 @@ async fn dispatch_mailbox_mutation(
other => return Disposition::Permanent(format!("unexpected mailbox mutation '{other}'")),
};
let folder_checkpoint = match db
.get_folder_sync_state(&email.account_id, &email.folder_id)
.await
{
Ok(checkpoint) => checkpoint,
Err(e) => return Disposition::Retry(format!("failed to read folder checkpoint: {e}")),
};
let backend = match env.mail_backend(&email.account_id).await {
Ok(backend) => backend,
Err(e) => return Disposition::Retry(format!("failed to build mail backend: {e}")),
};
// Recover the remote addressing from the stored message row -- the
// protocol-native id and the UIDVALIDITY scope it was captured under --
// rather than parsing it out of the opaque surrogate id. The backend still
// SELECT-verifies the UIDVALIDITY before mutating.
let spec = RemoteMutationSpec {
message_id: email.id.clone(),
remote_id: email.remote_id.clone(),
folder_id: email.folder_id.clone(),
folder_checkpoint,
uid_validity: email.uid_validity.clone(),
kind,
};
+9 -2
View File
@@ -628,6 +628,8 @@ mod tests {
id: id.to_string(),
account_id: "acct-mock-1".to_string(),
folder_id: folder_id.to_string(),
remote_id: id.to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "bob@nuncio.mx".to_string(),
@@ -864,11 +866,16 @@ mod tests {
// The wiremock stubs advertise 1 folder containing 1 message.
assert_eq!(synced, 1);
// The message persists under an opaque surrogate id; it is recovered by
// folder, and its protocol-native JMAP object id round-trips in
// `remote_id`.
let persisted = db
.get_message("jmap-msg-1")
.list_messages("inbox", 10)
.await
.expect("jmap message persisted");
assert_eq!(persisted.subject, "Welcome to JMAP Sync");
assert_eq!(persisted.len(), 1);
assert_eq!(persisted[0].subject, "Welcome to JMAP Sync");
assert_eq!(persisted[0].remote_id, "jmap-msg-1");
assert_eq!(
events.recv().await.expect("start event"),
@@ -43,6 +43,8 @@ fn sample_email(id: &str, subject: &str) -> Email {
id: id.to_string(),
account_id: ACCOUNT_ID.to_string(),
folder_id: "inbox".to_string(),
remote_id: id.to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "triage-e2e@nuncio.mx".to_string(),
@@ -103,9 +103,11 @@ fn sample_account() -> nuncio_core::AccountConfig {
fn sample_email(subject: &str) -> Email {
Email {
id: "imap-uid-42".to_string(),
id: "surrogate-inbox-42".to_string(),
account_id: ACCOUNT_ID.to_string(),
folder_id: FOLDER_ID.to_string(),
remote_id: "42".to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: "owner@nuncio.mx".to_string(),
@@ -161,11 +163,13 @@ async fn move_rule_executes_against_backend_and_completes_with_uidvalidity_check
let applied = env.backend.applied_mutations();
assert_eq!(applied.len(), 1, "the backend op must genuinely run");
assert_eq!(applied[0].message_id, "imap-uid-42");
// Addressing is recovered from the stored row's columns, not by parsing the
// opaque surrogate id: the protocol-native UID and the UIDVALIDITY scope it
// was captured under, so the backend can enforce its guard.
assert_eq!(applied[0].message_id, "surrogate-inbox-42");
assert_eq!(applied[0].remote_id, "42");
assert_eq!(applied[0].folder_id, FOLDER_ID);
// The source folder's stored checkpoint (carrying UIDVALIDITY) is threaded
// through so the backend can enforce its guard.
assert_eq!(applied[0].folder_checkpoint.as_deref(), Some(CHECKPOINT));
assert_eq!(applied[0].uid_validity, "1");
assert_eq!(
applied[0].kind,
RemoteMutationKind::Move {
+2
View File
@@ -77,6 +77,8 @@ fn mock_inbound_email(id: &str, subject: &str, body: &str) -> Email {
id: id.to_string(),
account_id: ACCOUNT_ID.to_string(),
folder_id: "inbox".to_string(),
remote_id: id.to_string(),
uid_validity: "1".to_string(),
subject: subject.to_string(),
sender: "alice@nuncio.mx".to_string(),
recipient: ACCOUNT_EMAIL.to_string(),