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)