From c660ed6d065d86d6e4462a482ecea83f27922a8f Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 31 Jul 2026 15:52:07 -0500 Subject: [PATCH 1/2] feat(cal): add CalendarEvent::to_ical serializer (#345) Adds the CalDAV write-back inverse of IcalParserAdapter::parse_ical: a ToIcal extension trait wraps a single VEVENT (UID, DTSTAMP, DTSTART, DTEND, SUMMARY, LOCATION, RRULE) in a VCALENDAR shell, escaping TEXT values per RFC 5545 3.3.11. Refs #310 Co-authored-by: James Maes --- crates/nuncio-cal/src/ical.rs | 182 ++++++++++++++++++++++++++++++++++ crates/nuncio-cal/src/lib.rs | 2 + 2 files changed, 184 insertions(+) create mode 100644 crates/nuncio-cal/src/ical.rs diff --git a/crates/nuncio-cal/src/ical.rs b/crates/nuncio-cal/src/ical.rs new file mode 100644 index 0000000..77d104f --- /dev/null +++ b/crates/nuncio-cal/src/ical.rs @@ -0,0 +1,182 @@ +//! iCalendar (RFC 5545) VEVENT serializer -- the write-back inverse of +//! [`crate::parser::IcalParserAdapter`]. +//! +//! [`CalendarEvent`] stores `start_time`/`end_time` as resolved UTC unix timestamps (the parser +//! already collapses any `TZID`/floating/`Z` form down to a single UTC instant), so this +//! serializer always emits `Z`-suffixed UTC `DATE-TIME` values. That is a lossless encoding of +//! what the model actually holds: re-parsing the emitted text yields the same UTC instant no +//! matter which form the original payload used. + +use chrono::{DateTime, Utc}; +use nuncio_core::model::CalendarEvent; + +/// Adds RFC 5545 serialization to [`CalendarEvent`]. +/// +/// `CalendarEvent` is defined in `nuncio-core`, so this is an extension trait rather than an +/// inherent impl -- Rust forbids inherent impls on types owned by another crate. Importing the +/// trait makes `event.to_ical()` read identically to an inherent method at call sites. +pub trait ToIcal { + /// Serialize this event as a complete iCalendar payload: a single `VEVENT` wrapped in a + /// `VCALENDAR`, the shape a CalDAV `PUT` request body expects. + fn to_ical(&self) -> String; +} + +impl ToIcal for CalendarEvent { + fn to_ical(&self) -> String { + let dtstamp = format_utc_datetime(Utc::now().timestamp()); + let dtstart = format_utc_datetime(self.start_time); + let dtend = format_utc_datetime(self.end_time); + + let mut vevent = String::new(); + vevent.push_str("BEGIN:VEVENT\r\n"); + vevent.push_str(&format!("UID:{}\r\n", escape_text(&self.id))); + vevent.push_str(&format!("DTSTAMP:{}\r\n", dtstamp)); + vevent.push_str(&format!("DTSTART:{}\r\n", dtstart)); + vevent.push_str(&format!("DTEND:{}\r\n", dtend)); + vevent.push_str(&format!("SUMMARY:{}\r\n", escape_text(&self.summary))); + if let Some(location) = &self.location { + vevent.push_str(&format!("LOCATION:{}\r\n", escape_text(location))); + } + if let Some(rrule) = &self.rrule { + // RRULE is a structured value (RFC 5545 3.3.10), not TEXT -- it must not go through + // TEXT escaping, and the parser reads it back verbatim via `properties().get`. + vevent.push_str(&format!("RRULE:{}\r\n", rrule)); + } + vevent.push_str("END:VEVENT\r\n"); + + let mut vcalendar = String::new(); + vcalendar.push_str("BEGIN:VCALENDAR\r\n"); + vcalendar.push_str("VERSION:2.0\r\n"); + vcalendar.push_str("PRODID:-//Nuncio//Calendar Engine//EN\r\n"); + vcalendar.push_str(&vevent); + vcalendar.push_str("END:VCALENDAR\r\n"); + vcalendar + } +} + +/// Format a UTC unix timestamp as an RFC 5545 `DATE-TIME` in UTC form (`YYYYMMDDTHHMMSSZ`). +fn format_utc_datetime(unix_ts: i64) -> String { + DateTime::::from_timestamp(unix_ts, 0) + .map(|dt| dt.format("%Y%m%dT%H%M%SZ").to_string()) + // `from_timestamp` only returns `None` for a timestamp outside chrono's representable + // range; falling back to the Unix epoch keeps this a total function without panicking, + // since a corrupt in-memory timestamp is not something this serializer can repair. + .unwrap_or_else(|| "19700101T000000Z".to_string()) +} + +/// Escape a `TEXT` value per RFC 5545 3.3.11: backslash, semicolon, and comma are escaped with a +/// leading backslash, and embedded newlines become the two-character literal `\n`. Order matters +/// -- backslash must be escaped first so the escaping added for the other characters is not +/// itself re-escaped. +fn escape_text(value: &str) -> String { + let mut escaped = String::with_capacity(value.len()); + for ch in value.chars() { + match ch { + '\\' => escaped.push_str("\\\\"), + ';' => escaped.push_str("\\;"), + ',' => escaped.push_str("\\,"), + '\n' => escaped.push_str("\\n"), + '\r' => {} + other => escaped.push(other), + } + } + escaped +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::parser::IcalParserAdapter; + + fn event( + summary: &str, + start_time: i64, + end_time: i64, + rrule: Option<&str>, + location: Option<&str>, + ) -> CalendarEvent { + CalendarEvent { + id: "evt-roundtrip".to_string(), + account_id: "acct-1".to_string(), + calendar_id: "cal-1".to_string(), + summary: summary.to_string(), + start_time, + end_time, + rrule: rrule.map(str::to_string), + location: location.map(str::to_string), + } + } + + fn roundtrip(original: &CalendarEvent) -> CalendarEvent { + let ics = original.to_ical(); + IcalParserAdapter::parse_ical( + &original.id, + &original.account_id, + &original.calendar_id, + &ics, + ) + .expect("serialized iCalendar must parse back") + } + + /// Proves the serializer is the true inverse of [`IcalParserAdapter::parse_ical`] across the + /// event shapes CalDAV write-back must support: a plain timed event, an all-day event + /// (whole-day UTC window), an event whose UTC instant originated from a `TZID` source (the + /// model has already resolved it to UTC, so round-tripping through `to_ical` must preserve + /// that same instant), and a recurring event carrying an `RRULE`. + #[test] + fn to_ical_roundtrips_through_parse_ical_for_all_event_shapes() { + let timed = event( + "Standup", + 1_709_294_400, + 1_709_298_000, + None, + Some("Room 5"), + ); + assert_eq!(roundtrip(&timed), timed); + + // Whole-day window: 2024-03-01T00:00:00Z .. 2024-03-01T23:59:59Z, matching the parser's + // own DATE-only convention (start of day / end of day). + let all_day = event("Conference", 1_709_251_200, 1_709_337_599, None, None); + assert_eq!(roundtrip(&all_day), all_day); + + // 2024-07-15 09:00 America/New_York (EDT, UTC-4) resolves to this UTC instant; the model + // only ever stores the resolved UTC epoch, so serializing and re-parsing must land on + // exactly the same instant even though the TZID itself is not retained. + let from_tzid = event("Standup EDT", 1_721_048_400, 1_721_052_000, None, None); + assert_eq!(roundtrip(&from_tzid), from_tzid); + + let recurring = event( + "Weekly Sync", + 1_709_294_400, + 1_709_298_000, + Some("FREQ=WEEKLY;BYDAY=MO"), + Some("Conference Room 101"), + ); + assert_eq!(roundtrip(&recurring), recurring); + } + + #[test] + fn to_ical_escapes_rfc5545_special_characters_in_text_fields() { + let special = event( + "Comma, semicolon; backslash\\ and\nnewline", + 1_709_294_400, + 1_709_298_000, + None, + Some("Loc; with, chars"), + ); + let ics = special.to_ical(); + assert!(ics.contains("SUMMARY:Comma\\, semicolon\\; backslash\\\\ and\\nnewline\r\n")); + assert!(ics.contains("LOCATION:Loc\\; with\\, chars\r\n")); + assert_eq!(roundtrip(&special), special); + } + + #[test] + fn to_ical_wraps_a_single_vevent_in_a_vcalendar_shell() { + let plain = event("Solo Event", 1_709_294_400, 1_709_298_000, None, None); + let ics = plain.to_ical(); + assert!(ics.starts_with("BEGIN:VCALENDAR\r\n")); + assert!(ics.contains("VERSION:2.0\r\n")); + assert!(ics.trim_end().ends_with("END:VCALENDAR")); + assert_eq!(ics.matches("BEGIN:VEVENT").count(), 1); + } +} diff --git a/crates/nuncio-cal/src/lib.rs b/crates/nuncio-cal/src/lib.rs index d9a72bf..5943a0a 100644 --- a/crates/nuncio-cal/src/lib.rs +++ b/crates/nuncio-cal/src/lib.rs @@ -5,6 +5,7 @@ pub mod backend; pub mod caldav; +pub mod ical; pub mod mock; pub mod nlp; pub mod parser; @@ -13,6 +14,7 @@ pub mod scheduling; pub use backend::CalendarBackend; pub use caldav::{CalDavAccountConfig, CalDavClient}; +pub use ical::ToIcal; pub use mock::MockCalendarBackend; pub use nlp::{NaturalLanguageScheduler, NlpError, ParsedSchedulingIntent}; pub use parser::{CalendarError, IcalParserAdapter}; -- 2.54.0 From 8b4e0384c2099d4355c32c51e5a7de8c1bd0e7fc Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 31 Jul 2026 15:52:22 -0500 Subject: [PATCH 2/2] feat(nunciod): single-instance advisory lock on the DB path Acquire an exclusive OS advisory lock on a sidecar file beside the database path (.lock) before opening the store, so a second nunciod pointed at the same database fails fast with an actionable error instead of both instances writing to the same SQLite file. Previously only an incidental gRPC port-bind collision offered any protection against this. Uses fs4 (flock on Unix, LockFileEx on Windows) since std has no portable file-lock primitive. The guard is held for the process lifetime and dropped at clean shutdown, releasing the lock; the OS also releases it automatically if the process dies uncleanly, so there is no PID-file staleness to reason about. Refs #291 --- Cargo.lock | 11 +++ Cargo.toml | 1 + crates/nunciod/Cargo.toml | 1 + crates/nunciod/src/lib.rs | 1 + crates/nunciod/src/lock.rs | 180 +++++++++++++++++++++++++++++++++++++ crates/nunciod/src/main.rs | 8 ++ 6 files changed, 202 insertions(+) create mode 100644 crates/nunciod/src/lock.rs diff --git a/Cargo.lock b/Cargo.lock index f94cbed..ad112c3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1314,6 +1314,16 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fs4" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8640e34b88f7652208ce9e88b1a37a2ae95227d84abec377ccd3c5cfeb141ed4" +dependencies = [ + "rustix 1.1.4", + "windows-sys 0.59.0", +] + [[package]] name = "fs_extra" version = "1.3.0" @@ -2636,6 +2646,7 @@ dependencies = [ "arc-swap", "async-trait", "bytes", + "fs4", "hex", "nuncio-cal", "nuncio-cli", diff --git a/Cargo.toml b/Cargo.toml index e2e4e76..a0ca121 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,7 @@ prost = "0.14" tonic-prost-build = "0.14" protoc-bin-vendored = "3.2" subtle = "2.6" +fs4 = "0.13" diff --git a/crates/nunciod/Cargo.toml b/crates/nunciod/Cargo.toml index a43ef22..d1ee17f 100644 --- a/crates/nunciod/Cargo.toml +++ b/crates/nunciod/Cargo.toml @@ -34,6 +34,7 @@ subtle.workspace = true hex.workspace = true bytes.workspace = true zeroize.workspace = true +fs4.workspace = true [dev-dependencies] nuncio-cli = { path = "../nuncio-cli" } diff --git a/crates/nunciod/src/lib.rs b/crates/nunciod/src/lib.rs index 9a10ad9..0f3ef2d 100644 --- a/crates/nunciod/src/lib.rs +++ b/crates/nunciod/src/lib.rs @@ -6,6 +6,7 @@ pub mod calendar_sync; pub mod contacts_sync; pub mod grpc; pub mod lifecycle; +pub mod lock; pub mod orchestrator; pub mod outbox; pub mod send; diff --git a/crates/nunciod/src/lock.rs b/crates/nunciod/src/lock.rs new file mode 100644 index 0000000..130eea5 --- /dev/null +++ b/crates/nunciod/src/lock.rs @@ -0,0 +1,180 @@ +//! Exclusive, cross-platform advisory lock on the daemon's database path. +//! +//! Nothing about starting a second `nunciod` process today prevents it from +//! opening the same SQLite database another instance already has open -- +//! the only accidental protection is a port-bind collision on the gRPC +//! listener, and that only fires if both instances also happen to pick the +//! same port. Two writers on one SQLite file corrupt state. +//! +//! [`InstanceLock::acquire`] takes an OS-level exclusive lock (`flock` on +//! Unix, `LockFileEx` on Windows via the `fs4` crate) on a sidecar file next +//! to the database, `.lock`. The lock is scoped to the OS file +//! handle, not tracked by a PID written into the file, so it cannot go +//! stale: if the holding process dies for any reason (crash, kill -9), the +//! OS releases the lock the moment the file descriptor/handle closes. + +use std::fs::{File, OpenOptions}; +use std::path::{Path, PathBuf}; + +use fs4::fs_std::FileExt; + +/// Failure to acquire the single-instance lock. +#[derive(Debug, thiserror::Error)] +pub enum InstanceLockError { + /// The lock is already held by another live process. The caller must + /// treat this as fatal and refuse to open the store. + #[error("another nunciod instance is already running for this database at {0}")] + AlreadyRunning(PathBuf), + + /// The lock file itself could not be created/opened/locked, e.g. a + /// permissions problem or an unwritable parent directory. + #[error("failed to acquire instance lock at {path}: {source}")] + Io { + path: PathBuf, + #[source] + source: std::io::Error, + }, +} + +/// Holds the exclusive lock on the database path for as long as this guard +/// is alive. Drop it (or let it fall out of scope at process exit) to +/// release the lock; a later call to [`InstanceLock::acquire`] for the same +/// path then succeeds again. +#[derive(Debug)] +pub struct InstanceLock { + file: File, + path: PathBuf, +} + +impl InstanceLock { + /// Acquires the exclusive lock for `db_path`, creating the sidecar lock + /// file (`.lock`) if it does not already exist. + /// + /// Returns [`InstanceLockError::AlreadyRunning`] if another process + /// already holds it -- callers must fail startup on this rather than + /// proceed to open the store, since a second writer on the same SQLite + /// file is exactly the corruption this guards against. + pub fn acquire(db_path: &Path) -> Result { + let lock_path = lock_path_for(db_path); + + if let Some(parent) = lock_path.parent() { + std::fs::create_dir_all(parent).map_err(|source| InstanceLockError::Io { + path: lock_path.clone(), + source, + })?; + } + + let file = OpenOptions::new() + .create(true) + .truncate(false) + .write(true) + .open(&lock_path) + .map_err(|source| InstanceLockError::Io { + path: lock_path.clone(), + source, + })?; + + let acquired = file + .try_lock_exclusive() + .map_err(|source| InstanceLockError::Io { + path: lock_path.clone(), + source, + })?; + + if !acquired { + return Err(InstanceLockError::AlreadyRunning(db_path.to_path_buf())); + } + + Ok(Self { + file, + path: lock_path, + }) + } +} + +impl Drop for InstanceLock { + fn drop(&mut self) { + // The OS releases a `flock`/`LockFileEx` lock automatically when the + // underlying handle closes, so a failure here is not a leaked lock + // (the process exiting, or `self.file` being dropped right after, + // clears it regardless) -- it is only logged so an operator sees + // anything unexpected in the unlock call itself. + if let Err(err) = self.file.unlock() { + tracing::warn!( + path = %self.path.display(), + error = %err, + "failed to explicitly release nunciod instance lock; it will still be \ + released when the process exits and the underlying file handle closes" + ); + } + } +} + +fn lock_path_for(db_path: &Path) -> PathBuf { + let mut file_name = db_path.as_os_str().to_owned(); + file_name.push(".lock"); + PathBuf::from(file_name) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn second_acquire_for_same_path_fails_while_first_guard_is_held() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let first = InstanceLock::acquire(&db_path).unwrap(); + + let second = InstanceLock::acquire(&db_path); + assert!( + matches!(second, Err(InstanceLockError::AlreadyRunning(ref p)) if p == &db_path), + "expected AlreadyRunning({db_path:?}), got {second:?}" + ); + + drop(first); + } + + #[test] + fn acquire_succeeds_again_after_the_first_guard_is_dropped() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let first = InstanceLock::acquire(&db_path).unwrap(); + drop(first); + + let second = InstanceLock::acquire(&db_path); + assert!( + second.is_ok(), + "acquiring after the prior guard dropped must succeed, got {:?}", + second.err() + ); + } + + #[test] + fn error_message_names_the_database_path() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let first = InstanceLock::acquire(&db_path).unwrap(); + let second = InstanceLock::acquire(&db_path).unwrap_err(); + + let message = second.to_string(); + assert!(message.contains("already running")); + assert!(message.contains(&db_path.display().to_string())); + + drop(first); + } + + #[test] + fn lock_file_is_created_beside_the_database_path() { + let dir = tempfile::tempdir().unwrap(); + let db_path = dir.path().join("nuncio.db"); + + let guard = InstanceLock::acquire(&db_path).unwrap(); + assert!(dir.path().join("nuncio.db.lock").exists()); + + drop(guard); + } +} diff --git a/crates/nunciod/src/main.rs b/crates/nunciod/src/main.rs index d50f90b..29bb348 100644 --- a/crates/nunciod/src/main.rs +++ b/crates/nunciod/src/main.rs @@ -46,6 +46,14 @@ async fn main() -> Result<(), Box> { .map(std::path::PathBuf::from) .unwrap_or_else(|_| nunciod::default_db_path()); + // Exclusive single-instance lock on the database path, acquired BEFORE + // the store is opened. Held for the entire process lifetime (dropped at + // the end of `main`, releasing it on any clean exit) so a second + // `nunciod` pointed at the same database fails fast instead of the two + // instances corrupting SQLite state between them. + let _instance_lock = + nunciod::lock::InstanceLock::acquire(&db_path).map_err(|e| format!("{e}"))?; + let orchestrator = nunciod::SelfHealingSyncOrchestrator::new(&db_path, event_bus.clone()); let (db, _summary) = orchestrator.initialize_and_recover().await?; -- 2.54.0