From 38e416b2b83522a5e3b615e2876799fd2dd50118 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:17:49 +0000 Subject: [PATCH] Fail closed when /schedule time math overflows or mktime fails. every hour used raw now+3600 (unlike every N hours / add_seconds), so a near-int64 last-fire could wrap to a negative next_fire_at. mktime failure returned -1, which list_due treats as always due and a recurring recompute would tight-loop. Return 0 / a parse error instead. Co-authored-by: Tyler Reckart --- CHANGELOG.md | 7 ++++ include/schedule_parser.h | 2 +- src/schedule_parser.cpp | 64 ++++++++++++++++++++++++++++------ tests/test_schedule_parser.cpp | 24 +++++++++++++ 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dae12b43..770d2e92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ loosely while pre-1.0 (breaking changes can land on minor bumps). ## [Unreleased] +### Fixed +- **`/schedule` time math fail-closed.** `every hour` / `hourly` now use the + same overflow-checked adder as `every N hours` instead of `now + 3600`. + `mktime` failure no longer stores `next_fire_at = -1` (always due on + `list_due`); parse and `next_fire_for_recur` return 0 / an error so a + recurring recompute cannot tight-loop. + ## [0.13.4] — 2026-09-10 ### Fixed diff --git a/include/schedule_parser.h b/include/schedule_parser.h index 640f6670..83fd22d2 100644 --- a/include/schedule_parser.h +++ b/include/schedule_parser.h @@ -53,7 +53,7 @@ ParseResult parse_schedule_phrase(const std::string& phrase, int64_t now); // Compute the next fire time for a recurring spec, given the last fire. // `recur_json` shape: -// {"every":"hour"} → +1h from after +// {"every":"hour"} → +1h from after (0 on overflow) // {"every":"day","at":"09:00"} → next 09:00 strictly after `after` // {"every":"week","day":"mon","at":"09:00"} → next Mon 09:00 strictly after `after` // {"every_minutes":N} → +Nm from after diff --git a/src/schedule_parser.cpp b/src/schedule_parser.cpp index d1cc9a96..9239e0d9 100644 --- a/src/schedule_parser.cpp +++ b/src/schedule_parser.cpp @@ -103,6 +103,12 @@ int64_t make_local_epoch(int y, int mo, int d, int hh, int mm) { tm.tm_sec = 0; tm.tm_isdst = -1; time_t t = std::mktime(&tm); + // POSIX uses (time_t)-1 for failure. list_due is `next_fire_at <= now`, + // so -1 is always due and a recurring recompute would tight-loop. + // 0 is the same sentinel next_fire_for_recur already uses for "cannot + // compute". A real timestamp of -1 (1969-12-31 23:59:59 UTC) is + // indistinguishable and is also rejected. + if (t == static_cast(-1)) return 0; return static_cast(t); } @@ -123,8 +129,9 @@ int64_t next_local_at(int64_t after, int hh, int mm) { int y, mo, d, h0, m0, w; local_date(after, y, mo, d, h0, m0, w); int64_t cand = make_local_epoch(y, mo, d, hh, mm); - if (cand <= after) cand = make_local_epoch(y, mo, d + 1, hh, mm); - return cand; + if (cand > after) return cand; + cand = make_local_epoch(y, mo, d + 1, hh, mm); + return cand > after ? cand : 0; } int64_t next_local_weekday_at(int64_t after, int target_wday, int hh, int mm) { @@ -132,8 +139,9 @@ int64_t next_local_weekday_at(int64_t after, int target_wday, int hh, int mm) { local_date(after, y, mo, d, h0, m0, w); int delta = (target_wday - w + 7) % 7; int64_t cand = make_local_epoch(y, mo, d + delta, hh, mm); - if (cand <= after) cand = make_local_epoch(y, mo, d + delta + 7, hh, mm); - return cand; + if (cand > after) return cand; + cand = make_local_epoch(y, mo, d + delta + 7, hh, mm); + return cand > after ? cand : 0; } std::string two(int n) { @@ -226,6 +234,8 @@ bool add_seconds(int64_t now, int64_t n, int64_t unit, int64_t& out) { return true; } +constexpr const char* kFireTimeErr = "could not compute local fire time"; + } // namespace ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { @@ -275,11 +285,16 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { if (std::regex_match(phrase, m, re)) { int hh, mm; if (parse_hhmm(m[1].str(), hh, mm)) { + const int64_t cand = next_local_at(now, hh, mm); + if (cand <= 0) { + r.error.message = kFireTimeErr; + return r; + } r.ok = true; r.spec.kind = ScheduleSpec::Kind::Once; - r.spec.fire_at = next_local_at(now, hh, mm); - r.spec.next_fire_at = r.spec.fire_at; - r.spec.normalized = "at " + format_local(r.spec.fire_at); + r.spec.fire_at = cand; + r.spec.next_fire_at = cand; + r.spec.normalized = "at " + format_local(cand); return r; } } @@ -298,6 +313,10 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { int y, mo, d, h0, m0, w; local_date(now, y, mo, d, h0, m0, w); int64_t cand = make_local_epoch(y, mo, d + 1, hh, mm); + if (cand <= 0) { + r.error.message = kFireTimeErr; + return r; + } r.ok = true; r.spec.kind = ScheduleSpec::Kind::Once; r.spec.fire_at = cand; @@ -323,6 +342,10 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { return r; } int64_t cand = make_local_epoch(y, mo, d, hh, mm); + if (cand <= 0) { + r.error.message = kFireTimeErr; + return r; + } if (cand <= now) { r.error.message = "scheduled time is in the past"; return r; @@ -338,10 +361,15 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { // ── "every hour" / "hourly" ───────────────────────────────────────── if (phrase == "every hour" || phrase == "hourly") { + int64_t next = 0; + if (!add_seconds(now, 1, 3600, next)) { + r.error.message = "interval too large"; + return r; + } r.ok = true; r.spec.kind = ScheduleSpec::Kind::Recurring; r.spec.recur_json = R"({"every":"hour"})"; - r.spec.next_fire_at = now + 3600; + r.spec.next_fire_at = next; r.spec.normalized = "every hour"; return r; } @@ -389,12 +417,17 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { r.error.message = "invalid HH:MM"; return r; } + const int64_t cand = next_local_at(now, hh, mm); + if (cand <= 0) { + r.error.message = kFireTimeErr; + return r; + } std::ostringstream js; js << "{\"every\":\"day\",\"at\":\"" << two(hh) << ":" << two(mm) << "\"}"; r.ok = true; r.spec.kind = ScheduleSpec::Kind::Recurring; r.spec.recur_json = js.str(); - r.spec.next_fire_at = next_local_at(now, hh, mm); + r.spec.next_fire_at = cand; std::ostringstream oss; oss << "every day at " << two(hh) << ":" << two(mm); r.spec.normalized = oss.str(); @@ -432,6 +465,11 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { r.error.message = "invalid HH:MM"; return r; } + const int64_t cand = next_local_weekday_at(now, wd, hh, mm); + if (cand <= 0) { + r.error.message = kFireTimeErr; + return r; + } std::ostringstream js; js << "{\"every\":\"week\",\"day\":\"" << lower(std::string(weekday_short(wd))) << "\",\"at\":\"" @@ -439,7 +477,7 @@ ParseResult parse_schedule_phrase(const std::string& phrase_in, int64_t now) { r.ok = true; r.spec.kind = ScheduleSpec::Kind::Recurring; r.spec.recur_json = js.str(); - r.spec.next_fire_at = next_local_weekday_at(now, wd, hh, mm); + r.spec.next_fire_at = cand; std::ostringstream oss; oss << "every " << weekday_short(wd) << " at " << two(hh) << ":" << two(mm); r.spec.normalized = oss.str(); @@ -472,7 +510,11 @@ int64_t next_fire_for_recur(const std::string& recur_json, int64_t after) { int hh = 9, mm = 0; if (!at.empty() && !parse_hhmm(at, hh, mm)) return 0; - if (every == "hour") return after + 3600; + if (every == "hour") { + int64_t next = 0; + if (!add_seconds(after, 1, 3600, next)) return 0; + return next; + } if (every == "day") return next_local_at(after, hh, mm); if (every == "week") { std::string day = parse_json_str_field(recur_json, "day"); diff --git a/tests/test_schedule_parser.cpp b/tests/test_schedule_parser.cpp index 4f3795a3..451b50ad 100644 --- a/tests/test_schedule_parser.cpp +++ b/tests/test_schedule_parser.cpp @@ -7,6 +7,7 @@ #include "schedule_parser.h" #include +#include #include using namespace arbiter; @@ -102,6 +103,14 @@ TEST_CASE("parse: 'on YYYY-MM-DD'") { auto past = parse_schedule_phrase("on 2020-01-01", now); CHECK(!past.ok); } + + SUBCASE("year 1 does not persist next_fire_at=-1") { + // mktime fails or yields a pre-epoch value; either way we fail + // closed instead of storing -1 (which list_due treats as always due). + auto r = parse_schedule_phrase("on 0001-01-01 at 09:00", now); + CHECK_FALSE(r.ok); + CHECK(r.spec.next_fire_at == 0); + } } TEST_CASE("parse: recurring shapes") { @@ -192,6 +201,13 @@ TEST_CASE("next_fire_for_recur: advances daily, hourly, weekly correctly") { R"({"every_minutes":200000000000000000})", now); CHECK(n == 0); } + + SUBCASE("every hour near int64 max returns 0 rather than wrapping") { + // `after + 3600` overflowed; same fail-closed contract as every_hours. + const int64_t huge = std::numeric_limits::max() - 100; + int64_t n = next_fire_for_recur(R"({"every":"hour"})", huge); + CHECK(n == 0); + } } TEST_CASE("parse: huge intervals fail closed without throwing") { @@ -217,6 +233,14 @@ TEST_CASE("parse: huge intervals fail closed without throwing") { CHECK(r.error.message.find("too large") != std::string::npos); } + SUBCASE("every hour near int64 max fails closed") { + const int64_t huge = std::numeric_limits::max() - 100; + auto r = parse_schedule_phrase("every hour", huge); + CHECK_FALSE(r.ok); + CHECK(r.error.message.find("too large") != std::string::npos); + CHECK(r.spec.next_fire_at == 0); + } + SUBCASE("ordinary values still parse") { auto r = parse_schedule_phrase("in 2 hours", now); CHECK(r.ok);