Skip to main content

max / goingson

Audit Run 34 cold spots: JMAP tracing, connection-string, calendar bug, cred cleanup - GO-34-1: instrument the JMAP layer (client/session/email) + two IMAP debug methods with #[instrument(skip_all)]; skip_all keeps tokens/passwords out of spans, only non-secret ids/counts recorded as fields - GO-33-3: build the SQLite pool from a real path via .filename() (no URI parsing of ?vfs=/?mode=); keep sqlite::memory: URL form only for :memory: - GO-34-2: fix events-calendar.js:285 ||/?: precedence so an event with endTimeEpoch set but endTime null no longer computes new Date(null) - GO-34-4: delete legacy SmtpClient::new() plaintext-password path (no callers) - GO-34-5: imap_client.rs let-else the non-local sync_state unwrap 868 tests green, clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-06 14:57 UTC
Commit: 1528b9e6018972cede228170fb7c4a9d201cf2db
Parent: fdc3083
7 files changed, +38 insertions, -19 deletions
@@ -25,7 +25,17 @@ use std::time::Duration;
25 25 pub async fn init_pool(database_path: Option<&str>) -> Result<SqlitePool, sqlx::Error> {
26 26 let path = database_path.unwrap_or("goingson.db");
27 27
28 - let options = SqliteConnectOptions::from_str(&format!("sqlite:{}", path))?
28 + // Build options from a real filesystem path via `.filename()` so a path
29 + // containing URI syntax (`?vfs=`, `?mode=memory`, `#frag`) is treated as a
30 + // literal file, never parsed as connection options. `:memory:` still needs
31 + // the URL form so sqlx sets its `in_memory` flag and the 5-connection pool
32 + // shares one database (tests rely on this).
33 + let base = if path == ":memory:" {
34 + SqliteConnectOptions::from_str("sqlite::memory:")?
35 + } else {
36 + SqliteConnectOptions::new().filename(path)
37 + };
38 + let options = base
29 39 .create_if_missing(true)
30 40 .foreign_keys(true)
31 41 .journal_mode(SqliteJournalMode::Wal)
@@ -282,7 +282,7 @@
282 282 // Positioned events
283 283 dayEvts.forEach(e => {
284 284 const startDate = new Date(e.startTimeEpoch || new Date(e.startTime).getTime());
285 - const endEpoch = e.endTimeEpoch || e.endTime ? new Date(e.endTime).getTime() : (startDate.getTime() + 3600000);
285 + const endEpoch = e.endTimeEpoch || (e.endTime ? new Date(e.endTime).getTime() : (startDate.getTime() + 3600000));
286 286 const endDate = new Date(endEpoch);
287 287
288 288 const startMinutes = startDate.getHours() * 60 + startDate.getMinutes();
@@ -172,6 +172,7 @@ impl ImapClient {
172 172 }
173 173
174 174 /// Fetch emails from a specific folder with debug info
175 + #[tracing::instrument(skip_all, fields(folder = %folder))]
175 176 pub async fn fetch_emails_from_folder_debug(
176 177 &self,
177 178 folder: &str,
@@ -354,7 +355,13 @@ impl ImapClient {
354 355 };
355 356
356 357 let uids: Vec<u32> = if use_uid {
357 - let state = sync_state.unwrap();
358 + // `use_uid` is only set true in the (Some, Some) uid_validity-match
359 + // arm above, so `sync_state` is guaranteed Some here. Bind it with
360 + // let-else to keep that invariant local — a future edit to the match
361 + // can't silently turn this into a panic.
362 + let Some(state) = &sync_state else {
363 + return Err("internal error: use_uid set without sync_state".to_string());
364 + };
358 365 let search_query = format!("UID {}:*", state.last_seen_uid.saturating_add(1));
359 366 debug.push(format!("uid_search: {}", search_query));
360 367
@@ -600,6 +607,7 @@ impl ImapClient {
600 607 }
601 608
602 609 /// Debug fetch - returns diagnostic info about what's in a folder
610 + #[tracing::instrument(skip_all, fields(folder = %folder))]
603 611 pub async fn debug_folder(&self, folder: &str) -> Result<String, String> {
604 612 let mut session = self.connect().await?;
605 613
@@ -153,22 +153,6 @@ pub struct SmtpClient {
153 153 }
154 154
155 155 impl SmtpClient {
156 - /// Create an SMTP client with password authentication.
157 - ///
158 - /// Note: Prefer `with_password()` for secure credential handling.
159 - /// This method reads the password from the account struct (legacy).
160 - pub fn new(account: &EmailAccount) -> Self {
161 - Self {
162 - server: account.smtp_server.clone(),
163 - port: account.smtp_port as u16,
164 - auth: SmtpAuth::Password {
165 - username: account.username.clone(),
166 - password: account.password.clone(),
167 - },
168 - from_address: account.email_address.clone(),
169 - }
170 - }
171 -
172 156 /// Create an SMTP client with explicit password authentication.
173 157 ///
174 158 /// Use this when retrieving credentials from secure storage (keychain).
@@ -3,6 +3,7 @@
3 3 use super::session::{discover_session, JmapSession};
4 4 use super::types::{JmapRequest, JmapResponse, MethodResponse};
5 5 use std::time::Duration;
6 + use tracing::instrument;
6 7
7 8 /// JMAP client for making API calls.
8 9 pub struct JmapClient {
@@ -56,6 +57,7 @@ impl JmapClient {
56 57 }
57 58
58 59 /// Gets or discovers the JMAP session.
60 + #[instrument(skip_all)]
59 61 pub async fn session(&mut self) -> Result<&JmapSession, String> {
60 62 if self.session.is_none() {
61 63 let session = discover_session(&self.session_url, &self.access_token).await?;
@@ -67,6 +69,7 @@ impl JmapClient {
67 69 }
68 70
69 71 /// Forces a session refresh.
72 + #[instrument(skip_all)]
70 73 pub async fn refresh_session(&mut self) -> Result<&JmapSession, String> {
71 74 let session = discover_session(&self.session_url, &self.access_token).await?;
72 75 self.session = Some(session);
@@ -97,6 +100,7 @@ impl JmapClient {
97 100 }
98 101
99 102 /// Executes a JMAP request and returns the response.
103 + #[instrument(skip_all)]
100 104 pub async fn execute(&mut self, request: JmapRequest) -> Result<JmapResponse, String> {
101 105 let api_url = self.api_url().await?;
102 106 // `apiUrl` is server-supplied; never POST the bearer token to a
@@ -138,6 +142,7 @@ impl JmapClient {
138 142 }
139 143
140 144 /// Executes a single method call and returns its response.
145 + #[instrument(skip_all, fields(jmap.method = %method))]
141 146 pub async fn call(
142 147 &mut self,
143 148 method: &str,
@@ -4,6 +4,7 @@ use chrono::{DateTime, Utc};
4 4 use super::client::JmapClient;
5 5 use super::types::{EmailFilter, JmapEmail, JmapRequest, SortCondition};
6 6 use serde_json::json;
7 + use tracing::instrument;
7 8
8 9 /// Parsed email for storage (similar to IMAP ParsedEmail).
9 10 #[derive(Debug, Clone)]
@@ -41,6 +42,7 @@ impl JmapClient {
41 42 /// * `mailbox_id` - The mailbox ID (use `inbox().await?.id` for inbox)
42 43 /// * `since` - Optional date filter for incremental sync
43 44 /// * `limit` - Maximum number of emails to fetch
45 + #[instrument(skip_all, fields(limit = limit))]
44 46 pub async fn fetch_emails(
45 47 &mut self,
46 48 mailbox_id: &str,
@@ -166,6 +168,7 @@ impl JmapClient {
166 168 }
167 169
168 170 /// Fetches emails from inbox.
171 + #[instrument(skip_all, fields(limit = limit))]
169 172 pub async fn fetch_inbox(
170 173 &mut self,
171 174 since: Option<DateTime<Utc>>,
@@ -176,6 +179,7 @@ impl JmapClient {
176 179 }
177 180
178 181 /// Fetches emails from archive.
182 + #[instrument(skip_all, fields(limit = limit))]
179 183 pub async fn fetch_archive(
180 184 &mut self,
181 185 since: Option<DateTime<Utc>>,
@@ -186,16 +190,19 @@ impl JmapClient {
186 190 }
187 191
188 192 /// Marks an email as read.
193 + #[instrument(skip_all, fields(email_id = %email_id))]
189 194 pub async fn mark_read(&mut self, email_id: &str) -> Result<(), String> {
190 195 self.set_keyword(email_id, "$seen", true).await
191 196 }
192 197
193 198 /// Marks an email as unread.
199 + #[instrument(skip_all, fields(email_id = %email_id))]
194 200 pub async fn mark_unread(&mut self, email_id: &str) -> Result<(), String> {
195 201 self.set_keyword(email_id, "$seen", false).await
196 202 }
197 203
198 204 /// Sets or removes a keyword on an email.
205 + #[instrument(skip_all, fields(email_id = %email_id, keyword = %keyword, set = set))]
199 206 async fn set_keyword(&mut self, email_id: &str, keyword: &str, set: bool) -> Result<(), String> {
200 207 let account_id = self.account_id().await?;
201 208
@@ -222,6 +229,7 @@ impl JmapClient {
222 229 }
223 230
224 231 /// Sends an email via JMAP Submission.
232 + #[instrument(skip_all)]
225 233 pub async fn send_email(
226 234 &mut self,
227 235 to: &str,
@@ -341,6 +349,7 @@ impl JmapClient {
341 349
342 350 /// Re-fetches a single email's full text body by JMAP id, without the 100KB
343 351 /// sync cap. Used to lazily load a body that was truncated during sync.
352 + #[instrument(skip_all, fields(jmap_id = %jmap_id))]
344 353 pub async fn fetch_email_full_body(&mut self, jmap_id: &str) -> Result<String, String> {
345 354 let account_id = self.account_id().await?;
346 355
@@ -368,6 +377,7 @@ impl JmapClient {
368 377 }
369 378
370 379 /// Tests JMAP connection by fetching session info.
380 + #[instrument(skip_all)]
371 381 pub async fn test_connection(session_url: &str, access_token: &str) -> Result<String, String> {
372 382 let mut client = JmapClient::new(session_url, access_token)?;
373 383 let session = client.session().await?;
@@ -4,6 +4,7 @@
4 4
5 5 use serde::Deserialize;
6 6 use std::collections::HashMap;
7 + use tracing::instrument;
7 8
8 9 /// JMAP session response from the session endpoint.
9 10 #[derive(Debug, Clone, Deserialize)]
@@ -72,6 +73,7 @@ pub fn require_https(url: &str) -> Result<(), String> {
72 73 }
73 74
74 75 /// Discovers the JMAP session for an account.
76 + #[instrument(skip_all)]
75 77 pub async fn discover_session(
76 78 session_url: &str,
77 79 access_token: &str,