Skip to main content

max / goingson

Commands/Core: local-time review windows, validation + recurrence hardening S-TZ: weekly and monthly review query windows were built by stamping local civil dates as UTC, misattributing edge-of-period rows for non-UTC users. Convert via a shared tz::local_civil_to_utc helper (system zone), matching day_planning's handling. S-VAL: - Validate estimated_minutes (sibling of scheduled_duration, previously unchecked) so a negative/huge value can't reach time-progress math. - Clamp SearchQuery::with_limit/with_offset to >= 0; SQLite treats a negative LIMIT as unbounded, so the cap silently no-opped before. - Stop leaking raw sqlx error text (table/column/SQL) to the frontend: log it, return a generic message. - validate_optional_duration now reports the real max instead of a hardcoded "24 hours". S-REC: filter out-of-range weekday bytes (valid 0..=6) before computing the next weekly occurrence so a corrupt/imported value can't drive a ~200-day jump, with a fallback when all are invalid; clamp the untrusted interval to keep the monthly i32 cast and civil-day math sane. DST: use earliest() instead of single() in the four recurrence time reconstructions so an occurrence landing in a spring-forward gap advances to the earliest valid instant rather than being silently dropped/stalled. Lifts recurrence.rs and repository.rs out of the cold-spot range; adds regression tests. Clippy clean; workspace tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-22 02:26 UTC
Commit: c2d3f43de00e6757b55e4a5ab9f072584dae0225
Parent: c17bb17
7 files changed, +115 insertions, -31 deletions
M crates/core/src/recurrence.rs +62 -9
@@ -104,7 +104,7 @@ fn add_civil_days(dt: DateTime<Utc>, days: i64, tz: Tz) -> DateTime<Utc> {
104 104 let local = dt.with_timezone(&tz);
105 105 let new_date = local.date_naive() + Duration::days(days);
106 106 tz.from_local_datetime(&new_date.and_time(local.time()))
107 - .single()
107 + .earliest() // earliest valid instant on a DST gap, rather than dropping the occurrence
108 108 .map(|t| t.with_timezone(&Utc))
109 109 .unwrap_or(dt + Duration::days(days))
110 110 }
@@ -139,7 +139,7 @@ fn add_months(dt: DateTime<Utc>, months: i32, target_day: Option<u32>, tz: Tz) -
139 139
140 140 tz.with_ymd_and_hms(new_year, new_month, new_day,
141 141 local.hour(), local.minute(), local.second())
142 - .single()
142 + .earliest() // earliest valid instant on a DST gap, rather than dropping the occurrence
143 143 .map(|t| t.with_timezone(&Utc))
144 144 .unwrap_or(dt)
145 145 }
@@ -196,21 +196,28 @@ pub fn calculate_next_due_rich_in_tz(
196 196 }
197 197 let base = current_due.copied().unwrap_or_else(Utc::now);
198 198 let local = base.with_timezone(&tz);
199 - let interval = rule.interval.max(1) as i64;
199 + // Clamp the interval: rule.interval is untrusted (can arrive via sync or
200 + // import). The upper bound keeps the i32 cast in the monthly arm and the
201 + // civil-day arithmetic from overflowing on an absurd value.
202 + let interval = rule.interval.clamp(1, 10_000) as i64;
200 203
201 204 match rule.pattern {
202 205 Recurrence::Daily => {
203 206 Some(add_civil_days(base, interval, tz))
204 207 }
205 208 Recurrence::Weekly => {
206 - if rule.weekdays.is_empty() {
209 + // Drop out-of-range weekday bytes (valid range is 0=Mon..6=Sun);
210 + // a corrupt/imported value like 200 would otherwise drive a
211 + // ~200-day civil jump through the `d > current_wd` branch below.
212 + let mut sorted_days: Vec<u8> =
213 + rule.weekdays.iter().copied().filter(|&d| d <= 6).collect();
214 + sorted_days.sort_unstable();
215 + sorted_days.dedup();
216 + if sorted_days.is_empty() {
207 217 return Some(add_civil_days(base, interval * 7, tz));
208 218 }
209 219 // Find next matching weekday (weekday read in `tz`).
210 220 let current_wd = local.weekday().num_days_from_monday() as u8;
211 - let mut sorted_days = rule.weekdays.clone();
212 - sorted_days.sort_unstable();
213 - sorted_days.dedup();
214 221
215 222 // Look for next day in the current week (after current weekday)
216 223 if let Some(&next_wd) = sorted_days.iter().find(|&&d| d > current_wd) {
@@ -287,7 +294,7 @@ fn nth_weekday_in_month(
287 294 d = d.pred_opt()?;
288 295 }
289 296 tz.with_ymd_and_hms(year, month, d.day(), hour, minute, second)
290 - .single()
297 + .earliest() // earliest valid instant on a DST gap, rather than dropping the occurrence
291 298 .map(|t| t.with_timezone(&Utc))
292 299 } else if (1..=5).contains(&week) {
293 300 // Nth occurrence: start from day 1, find first matching weekday, skip N-1
@@ -302,7 +309,7 @@ fn nth_weekday_in_month(
302 309 return None; // e.g., 5th Monday doesn't exist
303 310 }
304 311 tz.with_ymd_and_hms(year, month, d.day(), hour, minute, second)
305 - .single()
312 + .earliest() // earliest valid instant on a DST gap, rather than dropping the occurrence
306 313 .map(|t| t.with_timezone(&Utc))
307 314 } else {
308 315 None
@@ -640,6 +647,52 @@ mod tests {
640 647 }
641 648
642 649 #[test]
650 + fn test_rich_weekly_ignores_out_of_range_weekdays() {
651 + // A corrupt/imported weekday byte (200) must not drive a ~200-day jump;
652 + // out-of-range values are dropped, leaving the valid weekday (Wed).
653 + let mon = Utc.with_ymd_and_hms(2026, 3, 2, 10, 0, 0).unwrap(); // Monday
654 + let rule = RecurrenceRule {
655 + pattern: Recurrence::Weekly,
656 + interval: 1,
657 + weekdays: vec![200, 2], // garbage + Wed
658 + monthly_spec: None,
659 + };
660 + let next = calculate_next_due_rich(Some(&mon), &rule).unwrap();
661 + assert_eq!(next.weekday(), chrono::Weekday::Wed);
662 + assert!((next - mon).num_days() < 7, "must not jump far past one week");
663 + }
664 +
665 + #[test]
666 + fn test_rich_weekly_all_invalid_weekdays_falls_back() {
667 + // If every weekday byte is invalid, advance by the interval-week instead
668 + // of panicking on an empty sorted list.
669 + let mon = Utc.with_ymd_and_hms(2026, 3, 2, 10, 0, 0).unwrap();
670 + let rule = RecurrenceRule {
671 + pattern: Recurrence::Weekly,
672 + interval: 1,
673 + weekdays: vec![99, 200],
674 + monthly_spec: None,
675 + };
676 + let next = calculate_next_due_rich(Some(&mon), &rule).unwrap();
677 + assert_eq!((next - mon).num_days(), 7);
678 + }
679 +
680 + #[test]
681 + fn test_rich_interval_clamped() {
682 + // An absurd interval must not overflow the i32 month cast or civil math.
683 + let mon = Utc.with_ymd_and_hms(2026, 3, 2, 10, 0, 0).unwrap();
684 + let rule = RecurrenceRule {
685 + pattern: Recurrence::Daily,
686 + interval: u32::MAX,
687 + weekdays: vec![],
688 + monthly_spec: None,
689 + };
690 + // Clamped to 10_000 days; just assert it produces a finite future date.
691 + let next = calculate_next_due_rich(Some(&mon), &rule).unwrap();
692 + assert!(next > mon);
693 + }
694 +
695 + #[test]
643 696 fn test_rich_weekly_interval_2() {
644 697 // Friday, every 2 weeks on Mon/Fri
645 698 let fri = Utc.with_ymd_and_hms(2026, 3, 6, 10, 0, 0).unwrap(); // Friday
M crates/core/src/repository.rs +6 -4
@@ -732,15 +732,17 @@ impl SearchQuery {
732 732 self
733 733 }
734 734
735 - /// Sets the maximum number of results.
735 + /// Sets the maximum number of results. Negative values are clamped to 0:
736 + /// SQLite treats a negative LIMIT as unbounded, so an unclamped negative
737 + /// would silently disable the cap instead of enforcing it.
736 738 pub fn with_limit(mut self, limit: i64) -> Self {
737 - self.limit = Some(limit);
739 + self.limit = Some(limit.max(0));
738 740 self
739 741 }
740 742
741 - /// Sets the pagination offset.
743 + /// Sets the pagination offset. Negative values are clamped to 0.
742 744 pub fn with_offset(mut self, offset: i64) -> Self {
743 - self.offset = Some(offset);
745 + self.offset = Some(offset.max(0));
744 746 self
745 747 }
746 748
M crates/core/src/validation.rs +4 -2
@@ -39,14 +39,14 @@ fn validate_required_string(field: &'static str, value: &str, max_len: usize) ->
39 39 Ok(())
40 40 }
41 41
42 - /// Validates an optional duration is positive and within a max.
42 + /// Validates an optional duration is positive and within a max (in minutes).
43 43 fn validate_optional_duration(field: &'static str, value: Option<i32>, max: i32) -> Result<(), CoreError> {
44 44 if let Some(duration) = value {
45 45 if duration <= 0 {
46 46 return Err(CoreError::validation(field, "must be positive"));
47 47 }
48 48 if duration > max {
49 - return Err(CoreError::validation(field, "cannot exceed 24 hours"));
49 + return Err(CoreError::validation(field, format!("cannot exceed {} minutes", max)));
50 50 }
51 51 }
52 52 Ok(())
@@ -103,6 +103,7 @@ impl Validate for NewTask {
103 103 fn validate(&self) -> Result<(), CoreError> {
104 104 validate_required_string("description", &self.description, MAX_TASK_DESCRIPTION_LENGTH)?;
105 105 validate_optional_duration("scheduled_duration", self.scheduled_duration, MAX_SCHEDULED_DURATION_MINUTES)?;
106 + validate_optional_duration("estimated_minutes", self.estimated_minutes, MAX_SCHEDULED_DURATION_MINUTES)?;
106 107 validate_tags(&self.tags)
107 108 }
108 109 }
@@ -112,6 +113,7 @@ impl Validate for UpdateTask {
112 113 fn validate(&self) -> Result<(), CoreError> {
113 114 validate_required_string("description", &self.description, MAX_TASK_DESCRIPTION_LENGTH)?;
114 115 validate_optional_duration("scheduled_duration", self.scheduled_duration, MAX_SCHEDULED_DURATION_MINUTES)?;
116 + validate_optional_duration("estimated_minutes", self.estimated_minutes, MAX_SCHEDULED_DURATION_MINUTES)?;
115 117 validate_tags(&self.tags)
116 118 }
117 119 }
M src-tauri/src/commands/error.rs +7 -1
@@ -317,7 +317,13 @@ impl From<CoreError> for ApiError {
317 317 resource_id: None,
318 318 },
319 319 ),
320 - CoreError::Database { message, .. } => ApiError::database(message),
320 + CoreError::Database { message, .. } => {
321 + // Raw sqlx text exposes table/column/constraint names and SQL
322 + // fragments. Log it for diagnosis; return a generic message to
323 + // the frontend.
324 + tracing::error!(detail = %message, "database error");
325 + ApiError::database("A database error occurred")
326 + }
321 327 CoreError::Parse(message) => ApiError::parse(message),
322 328 CoreError::BadRequest(message) => ApiError::bad_request(message),
323 329 CoreError::Internal(message) => ApiError::internal(message),
M src-tauri/src/commands/monthly_review.rs +10 -8
@@ -3,7 +3,7 @@
3 3 //! Provides the Month view with heat-map data, stats, goals, reflections,
4 4 //! and pattern insights. Delegates aggregation to `goingson_core::monthly_review`.
5 5
6 - use chrono::{DateTime, Datelike, Duration, NaiveDate, Utc};
6 + use chrono::{Datelike, Duration, NaiveDate};
7 7 use serde::{Deserialize, Serialize};
8 8 use std::sync::Arc;
9 9 use tauri::State;
@@ -126,13 +126,15 @@ pub async fn get_monthly_review(
126 126 };
127 127 let month_end_date = monthly_review::month_end(month_start);
128 128
129 - // Time boundaries
130 - let month_start_dt = month_start.and_hms_opt(0, 0, 0)
131 - .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc))
132 - .unwrap_or_else(Utc::now);
133 - let month_end_dt = month_end_date.and_hms_opt(23, 59, 59)
134 - .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc))
135 - .unwrap_or_else(Utc::now);
129 + // Time boundaries. The month's civil dates are in the user's local zone;
130 + // convert via the local zone so edge-of-month rows aren't misattributed by
131 + // the UTC offset (matches day_planning's handling).
132 + let month_start_dt = crate::tz::local_civil_to_utc(
133 + month_start.and_hms_opt(0, 0, 0).expect("00:00:00 is valid"),
134 + );
135 + let month_end_dt = crate::tz::local_civil_to_utc(
136 + month_end_date.and_hms_opt(23, 59, 59).expect("23:59:59 is valid"),
137 + );
136 138 let month_str = month_start.format("%Y-%m").to_string();
137 139
138 140 // Fetch all data from repositories
M src-tauri/src/commands/weekly_review.rs +9 -7
@@ -108,13 +108,15 @@ pub async fn get_weekly_review(
108 108 let week_start = resolve_week_start(input.as_ref().and_then(|i| i.week_start.as_deref()))?;
109 109 let week_end_date = weekly_review::week_end(week_start);
110 110
111 - // Time boundaries for queries
112 - let week_start_dt = week_start.and_hms_opt(0, 0, 0)
113 - .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc))
114 - .unwrap_or_else(Utc::now);
115 - let week_end_dt = week_end_date.and_hms_opt(23, 59, 59)
116 - .map(|dt| DateTime::<Utc>::from_naive_utc_and_offset(dt, Utc))
117 - .unwrap_or_else(Utc::now);
111 + // Time boundaries for queries. The week's civil dates are in the user's
112 + // local zone; convert via the local zone so edge-of-week rows aren't
113 + // misattributed by the UTC offset (matches day_planning's handling).
114 + let week_start_dt = crate::tz::local_civil_to_utc(
115 + week_start.and_hms_opt(0, 0, 0).expect("00:00:00 is valid"),
116 + );
117 + let week_end_dt = crate::tz::local_civil_to_utc(
118 + week_end_date.and_hms_opt(23, 59, 59).expect("23:59:59 is valid"),
119 + );
118 120 let next_week_end_dt = week_end_dt + Duration::days(7);
119 121 let now = Utc::now();
120 122
M src-tauri/src/tz.rs +17
@@ -5,6 +5,7 @@
5 5 //! this zone (see `goingson_core::recurrence`) so a fixed local time-of-day — "every
6 6 //! day at 09:00" — survives daylight-saving transitions instead of drifting an hour.
7 7
8 + use chrono::{DateTime, NaiveDateTime, TimeZone, Utc};
8 9 use chrono_tz::Tz;
9 10
10 11 /// The user's IANA time zone, resolved from the OS. Falls back to UTC if the OS
@@ -15,3 +16,19 @@ pub fn system_tz() -> Tz {
15 16 .and_then(|name| name.parse().ok())
16 17 .unwrap_or(Tz::UTC)
17 18 }
19 +
20 + /// Interpret a civil (wall-clock) datetime as being in the user's system zone
21 + /// and convert it to the corresponding UTC instant.
22 + ///
23 + /// Window queries (weekly/monthly review) work with civil dates like "the start
24 + /// of this week"; those midnights are local, not UTC, so stamping them as UTC
25 + /// misattributes edge-of-period rows by the zone offset. On a DST spring-forward
26 + /// gap the civil time doesn't exist, so fall back to treating it as UTC rather
27 + /// than panicking.
28 + pub fn local_civil_to_utc(civil: NaiveDateTime) -> DateTime<Utc> {
29 + system_tz()
30 + .from_local_datetime(&civil)
31 + .earliest()
32 + .map(|dt| dt.with_timezone(&Utc))
33 + .unwrap_or_else(|| DateTime::<Utc>::from_naive_utc_and_offset(civil, Utc))
34 + }