From 63aa15d1559dc829918d6ee584cf5c7230d52bc2 Mon Sep 17 00:00:00 2001
From: Shaw <shawmakesmagic@gmail.com>
Date: Wed, 3 Jun 2026 01:12:07 -0400
Subject: [PATCH] fix(scheduler): fail closed on malformed scheduled_time
 instead of 500 (#1410)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

compute_next_run parsed scheduled_time as "HH:MM" with int(parts[0]),
int(parts[1]) and no validation, so "9", "9am", "25:00", "9:" or ":30" raised
IndexError/ValueError. The POST /tasks create route passes the user/LLM-supplied
scheduled_time before its try block (and only validates the cron field), so a
bad value surfaced as an unhandled 500 rather than the clean 400 used for other
invalid fields — and the same crash could fire inside the scheduler loop when
recomputing next_run for an already-stored bad row.

Guard the parse and fail closed (warn + return None), matching the existing
invalid-cron handling in the same function.

Adds tests/test_scheduler_scheduled_time_validation.py — malformed values return
None (fail before with IndexError/ValueError), valid HH:MM still computes.
---
 src/task_scheduler.py                         | 13 ++++++++--
 ...est_scheduler_scheduled_time_validation.py | 26 +++++++++++++++++++
 2 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 tests/test_scheduler_scheduled_time_validation.py

diff --git a/src/task_scheduler.py b/src/task_scheduler.py
index f67c214..f80c8a1 100644
--- a/src/task_scheduler.py
+++ b/src/task_scheduler.py
@@ -122,9 +122,18 @@ def compute_next_run(schedule: str, scheduled_time: str,
     if not scheduled_time:
         return None
 
-    # Parse HH:MM
+    # Parse HH:MM — fail closed on malformed input (no colon, non-numeric,
+    # out-of-range) the same way an invalid cron expression does above, so a
+    # bad value like "9" or "9am" returns None instead of raising IndexError/
+    # ValueError out of the create route (a 500) or the scheduler loop.
     parts = scheduled_time.split(":")
-    hour, minute = int(parts[0]), int(parts[1])
+    try:
+        hour, minute = int(parts[0]), int(parts[1])
+        if not (0 <= hour <= 23 and 0 <= minute <= 59):
+            raise ValueError("hour/minute out of range")
+    except (ValueError, IndexError):
+        logger.warning(f"Invalid scheduled_time '{scheduled_time}'")
+        return None
 
     if schedule == "daily":
         candidate = now.replace(hour=hour, minute=minute, second=0, microsecond=0)
diff --git a/tests/test_scheduler_scheduled_time_validation.py b/tests/test_scheduler_scheduled_time_validation.py
new file mode 100644
index 0000000..de1f3e6
--- /dev/null
+++ b/tests/test_scheduler_scheduled_time_validation.py
@@ -0,0 +1,26 @@
+"""Regression: compute_next_run must fail closed on a malformed scheduled_time.
+
+compute_next_run parsed scheduled_time as "HH:MM" with a bare
+`int(parts[0]), int(parts[1])` and no validation, so a value like "9", "9am",
+"25:00", "9:" or ":30" raised IndexError/ValueError. The POST /tasks create
+route calls it with the user/LLM-supplied scheduled_time *before* its try block
+(and only validates cron), so a bad value surfaced as an unhandled 500 instead
+of a clean 400 — and the same crash could fire inside the scheduler loop when
+recomputing next_run for an already-stored bad row.
+
+Now it fails closed (returns None) like an invalid cron expression does.
+"""
+from datetime import datetime
+
+from src.task_scheduler import compute_next_run
+
+
+def test_malformed_scheduled_time_returns_none():
+    now = datetime(2026, 6, 2, 12, 0)
+    for bad in ("9", "9am", "09", "9:", ":30", "abc", "25:00", "09:99", ""):
+        assert compute_next_run("daily", bad, after=now) is None, bad
+
+
+def test_valid_scheduled_time_still_computes():
+    now = datetime(2026, 6, 2, 8, 0)
+    assert compute_next_run("daily", "09:00", after=now) == datetime(2026, 6, 2, 9, 0)