Merge pull request #999 from scitran/rule-regex-validation

Add rule regex input validation

api/jobs/handlers.py

@@ -25,6 +25,7 @@ from .gears import validate_gear_config, get_gears, get_gear, get_invocation_sch
 from .jobs import Job, Logs
 from .batch import check_state, update
 from .queue import Queue
+from .rules import validate_regexes
 
 
 class GearsHandler(base.RequestHandler):
@@ -176,6 +177,7 @@ class RulesHandler(base.RequestHandler):
         doc = self.request.json
 
         validate_data(doc, 'rule-new.json', 'input', 'POST', optional=True)
+        validate_regexes(doc)
         try:
             get_gear_by_name(doc['alg'])
         except APINotFoundException:
@@ -228,6 +230,7 @@ class RuleHandler(base.RequestHandler):
 
         updates = self.request.json
         validate_data(updates, 'rule-update.json', 'input', 'POST', optional=True)
+        validate_regexes(updates)
         if updates.get('alg'):
             try:
                 get_gear_by_name(updates['alg'])

api/jobs/rules.py

@@ -3,6 +3,7 @@ import re
 
 from .. import config
 from ..types import Origin
+from ..dao import APIValidationException
 from ..dao.containerutil import FileReference
 
 from . import gears
@@ -288,4 +289,17 @@ def copy_site_rules_for_project(project_id):
         config.db.project_rules.insert_one(doc)
 
 
-
+def validate_regexes(rule):
+    invalid_patterns = set()
+    for match in rule.get('all', []) + rule.get('any', []):
+        if match.get('regex'):
+            pattern = match['value']
+            try:
+                re.compile(pattern)
+            except re.error:
+                invalid_patterns.add(pattern)
+    if invalid_patterns:
+        raise APIValidationException({
+            'reason': 'Cannot compile regex patterns',
+            'patterns': sorted(invalid_patterns),
+        })

tests/integration_tests/python/test_rules.py

@@ -31,7 +31,22 @@ def test_site_rules(randstr, data_builder, as_admin, as_user, as_public):
     assert r.status_code == 403
 
     # attempt to add site rule with empty payload
-    r = as_admin.post('site/rules', json={})
+    r = as_admin.post('/site/rules', json={})
+    assert r.status_code == 400
+    assert 'Empty Payload' in r.json()['message']
+
+    # attempt to add site rule with invalid regex
+    invalid_pattern = '^(?non-image$).+'
+    r = as_admin.post('/site/rules', json={
+        'alg': gear_name,
+        'name': 'invalid-regex-rule',
+        'any': [],
+        'all': [
+            {'type': 'file.measurements', 'value': invalid_pattern, 'regex': True},
+        ]
+    })
+    assert r.status_code == 422
+    assert invalid_pattern in r.json()['patterns']
 
     # add site rule
     r = as_admin.post('/site/rules', json=rule)
@@ -83,6 +98,15 @@ def test_site_rules(randstr, data_builder, as_admin, as_user, as_public):
     r = as_admin.put('/site/rules/' + rule_id, json={})
     assert r.status_code == 400
 
+    # attempt to modify site rule with invalid regex
+    r = as_admin.put('/site/rules/' + rule_id, json={
+        'all': [
+            {'type': 'file.measurements', 'value': invalid_pattern, 'regex': True},
+        ]
+    })
+    assert r.status_code == 422
+    assert invalid_pattern in r.json()['patterns']
+
     # modify site rule
     r = as_admin.put('/site/rules/' + rule_id, json=update)
     assert r.ok