fix(controller): validate config keys at the api level

Author: benwilber

controller/api/serializers.py

@@ -21,6 +21,7 @@
 CPUSHARE_MATCH = re.compile(r'^(?P<cpu>[0-9]+)$')
 TAGKEY_MATCH = re.compile(r'^[a-z]+$')
 TAGVAL_MATCH = re.compile(r'^\w+$')
+CONFIGKEY_MATCH = re.compile(r'^[a-z_]+[a-z0-9_]*$', re.IGNORECASE)
 
 
 class JSONFieldSerializer(serializers.Field):
@@ -181,6 +182,14 @@ class Meta:
         """Metadata options for a :class:`ConfigSerializer`."""
         model = models.Config
 
+    def validate_values(self, value):
+        for k, v in value.viewitems():
+            if not re.match(CONFIGKEY_MATCH, k):
+                raise serializers.ValidationError(
+                    "Config keys must start with a letter or underscore and "
+                    "only contain [A-z0-9_]")
+        return value
+
     def validate_memory(self, value):
         for k, v in value.viewitems():
             if v is None:  # use NoneType to unset a value

controller/api/tests/test_config.py

@@ -257,6 +257,41 @@ def test_config_str(self):
         config = Config.objects.get(uuid=config5['uuid'])
         self.assertEqual(str(config), "{}-{}".format(config5['app'], config5['uuid'][:7]))
 
+    @mock.patch('requests.post', mock_status_ok)
+    def test_valid_config_keys(self):
+        """Test that valid config keys are accepted.
+        """
+        keys = ("FOO", "_foo", "f001", "FOO_BAR_BAZ_")
+        url = '/v1/apps'
+        response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 201)
+        app_id = response.data['id']
+        url = '/v1/apps/{app_id}/config'.format(**locals())
+        for k in keys:
+            body = {'values': json.dumps({k: "testvalue"})}
+            resp = self.client.post(
+                url, json.dumps(body), content_type='application/json',
+                HTTP_AUTHORIZATION='token {}'.format(self.token))
+            self.assertEqual(resp.status_code, 201)
+            self.assertIn(k, resp.data['values'])
+
+    @mock.patch('requests.post', mock_status_ok)
+    def test_invalid_config_keys(self):
+        """Test that invalid config keys are rejected.
+        """
+        keys = ("123", "../../foo", "FOO/", "FOO-BAR")
+        url = '/v1/apps'
+        response = self.client.post(url, HTTP_AUTHORIZATION='token {}'.format(self.token))
+        self.assertEqual(response.status_code, 201)
+        app_id = response.data['id']
+        url = '/v1/apps/{app_id}/config'.format(**locals())
+        for k in keys:
+            body = {'values': json.dumps({k: "testvalue"})}
+            resp = self.client.post(
+                url, json.dumps(body), content_type='application/json',
+                HTTP_AUTHORIZATION='token {}'.format(self.token))
+            self.assertEqual(resp.status_code, 400)
+
     @mock.patch('requests.post', mock_status_ok)
     def test_admin_can_create_config_on_other_apps(self):
         """If a non-admin creates an app, an administrator should be able to set config