From ccd30ac239c620022f6d2aeeca32a721a728f963 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Fri, 7 Feb 2020 11:38:50 +0200 Subject: [PATCH] Stricter cron validation, reject schedules like "At midnight of February 31" --- CHANGELOG.md | 1 + hc/api/tests/test_update_check.py | 16 +++++++++++++--- hc/front/tests/test_update_timeout.py | 2 +- hc/front/validators.py | 5 ++++- hc/lib/jsonschema.py | 5 ++++- hc/lib/tests/test_jsonschema.py | 6 ++++-- requirements.txt | 2 +- 7 files changed, 28 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21f0e176..78dc806e 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/hc/api/tests/test_update_check.py b/hc/api/tests/test_update_check.py index 1e5076a8..d2a79fdd 100644 --- a/hc/api/tests/test_update_check.py +++ b/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 * * * *") diff --git a/hc/front/tests/test_update_timeout.py b/hc/front/tests/test_update_timeout.py index 2404251e..d5f5f7b8 100644 --- a/hc/front/tests/test_update_timeout.py +++ b/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} diff --git a/hc/front/validators.py b/hc/front/validators.py index 0b5fc8f2..6acb6b44 100644 --- a/hc/front/validators.py +++ b/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) diff --git a/hc/lib/jsonschema.py b/hc/lib/jsonschema.py index d7341c9c..d8d1f0b0 100644 --- a/hc/lib/jsonschema.py +++ b/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: diff --git a/hc/lib/tests/test_jsonschema.py b/hc/lib/tests/test_jsonschema.py index b83b95b6..302d6cb7 100644 --- a/hc/lib/tests/test_jsonschema.py +++ b/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): diff --git a/requirements.txt b/requirements.txt index f16014c4..e4fb0a78 100644 --- a/requirements.txt +++ b/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 \ No newline at end of file +statsd==3.3.0