Stricter cron validation, reject schedules like "At midnight of February 31"

CHANGELOG.md

@@ -20,6 +20,7 @@ All notable changes to this project will be documented in this file.
 - Make sure Check.last_ping and Ping.created timestamps match exactly
 - Don't trigger "down" notifications when changing schedule interactively in web UI
 - Fix sendalerts crash loop when encountering a bad cron schedule
+- Stricter cron validation, reject schedules like "At midnight of February 31"
 
 ## v1.12.0 - 2020-01-02
 

hc/api/tests/test_update_check.py

@@ -166,10 +166,20 @@ class UpdateCheckTestCase(BaseTestCase):
         self.assertEqual(r.status_code, 400)
 
     def test_it_rejects_non_string_desc(self):
-        r = self.post(
-            self.check.code, {"api_key": "X" * 32, "desc": 123}
-        )
+        r = self.post(self.check.code, {"api_key": "X" * 32, "desc": 123})
 
         self.assertEqual(r.status_code, 400)
 
+    def test_it_validates_cron_expression(self):
+        self.check.kind = "cron"
+        self.check.schedule = "5 * * * *"
+        self.check.save()
+
+        samples = ["* invalid *", "1,2 3,* * * *", "0 0 31 2 *"]
+        for sample in samples:
+            r = self.post(self.check.code, {"api_key": "X" * 32, "schedule": sample})
+            self.assertEqual(r.status_code, 400, "Did not reject '%s'" % sample)
+
+        # Schedule should be unchanged
         self.check.refresh_from_db()
+        self.assertEqual(self.check.schedule, "5 * * * *")

hc/front/tests/test_update_timeout.py

@@ -74,7 +74,7 @@ class UpdateTimeoutTestCase(BaseTestCase):
 
     def test_it_validates_cron_expression(self):
         self.client.login(username="alice@example.org", password="password")
-        samples = ["* invalid *", "1,2 3,* * * *"]
+        samples = ["* invalid *", "1,2 3,* * * *", "0 0 31 2 *"]
 
         for sample in samples:
             payload = {"kind": "cron", "schedule": sample, "tz": "UTC", "grace": 60}

hc/front/validators.py

@@ -25,7 +25,10 @@ class CronExpressionValidator(object):
             raise ValidationError(message=self.message)
 
         try:
-            croniter(value)
+            # Does croniter accept the schedule?
+            it = croniter(value)
+            # Can it calculate the next datetime?
+            it.next()
         except:
             raise ValidationError(message=self.message)
 

hc/lib/jsonschema.py

@@ -22,7 +22,10 @@ def validate(obj, schema, obj_name="value"):
             raise ValidationError("%s is too long" % obj_name)
         if schema.get("format") == "cron":
             try:
-                croniter(obj)
+                # Does croniter accept the schedule?
+                it = croniter(obj)
+                # Can it calculate the next datetime?
+                it.next()
             except:
                 raise ValidationError("%s is not a valid cron expression" % obj_name)
         if schema.get("format") == "timezone" and obj not in all_timezones:

hc/lib/tests/test_jsonschema.py

@@ -74,8 +74,10 @@ class JsonSchemaTestCase(TestCase):
             validate("baz", {"enum": ["foo", "bar"]})
 
     def test_it_checks_cron_format(self):
-        with self.assertRaises(ValidationError):
-            validate("x * * * *", {"type": "string", "format": "cron"})
+        samples = ["x * * * *", "0 0 31 2 *"]
+        for sample in samples:
+            with self.assertRaises(ValidationError):
+                validate(sample, {"type": "string", "format": "cron"})
 
     def test_it_checks_timezone_format(self):
         with self.assertRaises(ValidationError):

requirements.txt

@@ -4,4 +4,4 @@ django-compressor==2.4
 psycopg2==2.8.4
 pytz==2019.3
 requests==2.22.0
-statsd==3.3.0
+statsd==3.3.0