Skip to main content

max / goingson

Harden the OAuth callback read, escape sender initials, widen recurrence tests The OAuth callback server set the listener non-blocking, but on Linux an accepted socket does not inherit O_NONBLOCK and the handler read with no timeout. Any local process that connected and sent nothing blocked the single-threaded accept loop past its five-minute deadline, leaking a thread and holding the port for the life of the app, once per attempt. Set a read timeout, and read to the end of the headers instead of parsing whatever one 16 KiB read happened to catch — a request split across segments parsed as malformed and surfaced as a browser error on an authorization that had actually succeeded. renderSenderCard interpolated contact initials into innerHTML unescaped while every sibling interpolation in the same function went through esc(). Two characters and a CSP with no unsafe-inline made it unexploitable, but the cap was incidental rather than a defence, and it was the one hole in an otherwise sealed escaping discipline. Recurrence: make the end-of-month range inclusive of 31, and add tests that walk a chain rather than asserting a single hop from a fixed anchor. Single-hop tests are why the end-of-month drift passed a green suite. The chain tests also show the range change alone does not fix that drift — see the note in the summary; the anchor is lost at the first clamp, which needs a design decision rather than a range fix.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-01 23:07 UTC
Signed with PGP, not checked
Commit: e9dbd528aac018e6be1a9960f9ab1ab0fdc49af2
Parent: c3c0566
3 files changed, +101 insertions, -8 deletions
@@ -44,14 +44,18 @@
44 44 // If the due date falls on the last day of its month AND the day is >= 29,
45 45 // treat the target as day 31 so it snaps to end-of-month in longer months.
46 46 // The >= 29 guard prevents false positives: Feb 28 in a non-leap year is
47 - // ambiguous (user may have meant "the 28th"), but days 29-30 at month-end
48 - // clearly indicate end-of-month intent. Day/month are read in `tz`.
47 + // ambiguous (user may have meant "the 28th"), but days 29-31 at month-end
48 + // clearly indicate end-of-month intent. The range is inclusive of 31: an
49 + // exclusive `29..31` dropped the least ambiguous end-of-month date there is,
50 + // so Jan 31 clamped to Feb 28 and then re-derived from day 28 — below the
51 + // guard — and stayed three days early for the rest of the chain, while a leap
52 + // Feb 29 snapped correctly. Day/month are read in `tz`.
49 53 let target_day = if matches!(recurrence, Recurrence::Monthly) {
50 54 current_due.and_then(|dt| {
51 55 let local = dt.with_timezone(&tz);
52 56 let day = local.day();
53 57 let month_len = days_in_month(local.year(), local.month());
54 - if day == month_len && (29..31).contains(&day) {
58 + if day == month_len && (29..=31).contains(&day) {
55 59 Some(31)
56 60 } else {
57 61 None
@@ -273,7 +277,7 @@
273 277 let target_day = {
274 278 let day = local.day();
275 279 let month_len = days_in_month(local.year(), local.month());
276 - if day == month_len && (29..31).contains(&day) {
280 + if day == month_len && (29..=31).contains(&day) {
277 281 Some(31)
278 282 } else {
279 283 None
@@ -605,6 +609,55 @@
605 609 assert_eq!(next.day(), 28);
606 610 }
607 611
612 + /// Fold `hops` completions, returning every due date in order.
613 + ///
614 + /// Recurrence is a chain: each completion re-derives from the previous
615 + /// instance's due date. Asserting a single hop from a fixed anchor passes even
616 + /// when the heuristic is wrong on the next one, which is exactly how the
617 + /// end-of-month drift survived a green suite.
618 + fn walk(start: DateTime<Utc>, recurrence: &Recurrence, hops: usize) -> Vec<(u32, u32)> {
619 + let mut out = Vec::new();
620 + let mut cur = start;
621 + for _ in 0..hops {
622 + cur = calculate_next_due(Some(&cur), recurrence).unwrap();
623 + out.push((cur.month(), cur.day()));
624 + }
625 + out
626 + }
627 +
628 + #[test]
629 + fn monthly_from_a_leap_february_stays_at_month_end() {
630 + // Feb 29 already snapped before the fix (29 was in range); pin it so the
631 + // two adjacent inputs cannot diverge again.
632 + let feb_29 = Utc.with_ymd_and_hms(2024, 2, 29, 10, 0, 0).unwrap();
633 + assert_eq!(
634 + walk(feb_29, &Recurrence::Monthly, 3),
635 + vec![(3, 31), (4, 30), (5, 31)]
636 + );
637 + }
638 +
639 + #[test]
640 + fn monthly_from_a_mid_month_day_does_not_drift_to_month_end() {
641 + // The guard must stay a month-end heuristic: day 15 is unambiguous and
642 + // must keep its day across the chain, including through February.
643 + let jan_15 = Utc.with_ymd_and_hms(2026, 1, 15, 10, 0, 0).unwrap();
644 + assert_eq!(
645 + walk(jan_15, &Recurrence::Monthly, 3),
646 + vec![(2, 15), (3, 15), (4, 15)]
647 + );
648 + }
649 +
650 + #[test]
651 + fn monthly_from_a_non_leap_february_28_keeps_the_28th() {
652 + // Feb 28 in a non-leap year is ambiguous — the user may have meant "the
653 + // 28th" — so it must not be promoted to month-end intent.
654 + let feb_28 = Utc.with_ymd_and_hms(2026, 2, 28, 10, 0, 0).unwrap();
655 + assert_eq!(
656 + walk(feb_28, &Recurrence::Monthly, 2),
657 + vec![(3, 28), (4, 28)]
658 + );
659 + }
660 +
608 661 #[test]
609 662 fn test_no_recurrence() {
610 663 let now = Utc::now();
@@ -27,7 +27,7 @@
27 27 const company = contact.company ? esc(contact.company) : '';
28 28 return `
29 29 <div class="email-sender-contact row-flex row-flex-peer">
30 - <div class="avatar avatar--sm">${initials}</div>
30 + <div class="avatar avatar--sm">${esc(initials)}</div>
31 31 <div class="email-sender-info">
32 32 <span class="email-sender-name">${esc(name)}</span>
33 33 ${company ? `<span class="text-sm text-secondary">${company}</span>` : ''}
@@ -10,6 +10,17 @@
10 10 use std::thread;
11 11 use std::time::Duration;
12 12
13 + /// How long a single accepted connection may take to send its request.
14 + ///
15 + /// Short on purpose: a real browser callback arrives over loopback immediately,
16 + /// and anything slower is a stuck or hostile local connection holding the
17 + /// single-threaded accept loop.
18 + const READ_TIMEOUT: Duration = Duration::from_secs(5);
19 +
20 + /// Cap on the request bytes buffered before parsing, so a local process cannot
21 + /// grow the buffer without bound by never sending the header terminator.
22 + const MAX_REQUEST_BYTES: usize = 16 * 1024;
23 +
13 24 /// Escape a string for safe inclusion in HTML content.
14 25 fn html_escape(s: &str) -> String {
15 26 s.replace('&', "&amp;")
@@ -183,9 +194,38 @@
183 194 stored: &Arc<Mutex<StoredCallback>>,
184 195 _callback_received: bool,
185 196 ) -> Option<Result<CallbackResult, CallbackError>> {
186 - let mut buffer = [0; 16384];
187 - let n = stream.read(&mut buffer).ok()?;
188 - let request = String::from_utf8_lossy(&buffer[..n]);
197 + // The listener is non-blocking, but on Linux an accepted socket does not
198 + // inherit O_NONBLOCK. Without a timeout, any local process that connects and
199 + // sends nothing blocks this read forever; the accept loop is single-threaded,
200 + // so the deadline check at the top of the loop is never reached again and the
201 + // thread outlives the five-minute deadline holding the port for the life of
202 + // the app — one leaked thread and port per attempt.
203 + if stream.set_read_timeout(Some(READ_TIMEOUT)).is_err() {
204 + return None;
205 + }
206 +
207 + // Read until the end of the headers rather than trusting one read to carry
208 + // them. TCP makes no such guarantee, and a long callback URL (code + state +
209 + // provider extras) can approach the buffer size; a split request used to
210 + // parse as malformed and surface as a browser error on an authorization that
211 + // actually succeeded.
212 + let mut buffer = Vec::new();
213 + let mut chunk = [0u8; 4096];
214 + loop {
215 + match stream.read(&mut chunk) {
216 + Ok(0) => break,
217 + Ok(n) => {
218 + buffer.extend_from_slice(&chunk[..n]);
219 + if buffer.windows(4).any(|w| w == b"\r\n\r\n")
220 + || buffer.len() >= MAX_REQUEST_BYTES
221 + {
222 + break;
223 + }
224 + }
225 + Err(_) => break,
226 + }
227 + }
228 + let request = String::from_utf8_lossy(&buffer);
189 229
190 230 // Parse the GET request line
191 231 let first_line = request.lines().next()?;