Merge pull request #4709 from StackStorm/issue/disallowDecryptKVForValuesMarkedSecret

Disallow decrypt_kv jinja filter for fields marked as secret for pack config

Author: m4dcoder

CHANGELOG.rst

@@ -9,6 +9,8 @@ Changed
 
 * Allow the orquesta st2kv function to return default for nonexistent key. (improvement) #4678
 * Update requests library to latest version (2.22.0) in requirements. (improvement) #4680
+* Disallow "decrypt_kv" filter to be specified in the config for values that are marked as
+  "secret: True" in the schema. (improvement) #4709
 
 Fixed
 ~~~~~

st2api/st2api/controllers/v1/pack_configs.py

@@ -107,6 +107,8 @@ def put(self, pack_config_content, pack_ref, requester_user, show_secrets=False)
             config_api.validate(validate_against_schema=True)
         except jsonschema.ValidationError as e:
             raise ValueValidationException(six.text_type(e))
+        except ValueValidationException as e:
+            raise ValueValidationException(six.text_type(e))
 
         self._dump_config_to_disk(config_api)
 

st2api/tests/unit/controllers/v1/test_pack_configs.py

@@ -18,7 +18,6 @@
 
 from st2tests.api import FunctionalTest
 from st2api.controllers.v1.pack_configs import PackConfigsController
-
 from st2tests.fixturesloader import get_fixtures_packs_base_path
 
 __all__ = [
@@ -79,3 +78,16 @@ def test_put_pack_config(self):
                                           get_resp.json['values'], expect_errors=True)
         self.assertEqual(put_resp.status_int, 200)
         self.assertEqual(get_resp.json, put_resp_undo.json)
+
+    @mock.patch.object(PackConfigsController, '_dump_config_to_disk', mock.MagicMock())
+    def test_put_invalid_pack_config(self):
+        get_resp = self.app.get('/v1/configs/dummy_pack_11', params={'show_secrets': True},
+                                expect_errors=True)
+        config = copy.copy(get_resp.json['values'])
+        put_resp = self.app.put_json('/v1/configs/dummy_pack_11', config, expect_errors=True)
+        self.assertEqual(put_resp.status_int, 400)
+        expected_msg = ('Values specified as "secret: True" in config schema are automatically '
+                        'decrypted by default. Use of "decrypt_kv" jinja filter is not allowed '
+                        'for such values. Please check the specified values in the config or '
+                        'the default values in the schema.')
+        self.assertTrue(expected_msg in put_resp.json['faultstring'])

st2common/st2common/util/pack.py

@@ -25,6 +25,8 @@
 from st2common.constants.pack import PACK_REF_WHITELIST_REGEX
 from st2common.content.loader import MetaLoader
 from st2common.persistence.pack import Pack
+from st2common.exceptions.apivalidation import ValueValidationException
+from st2common.util import jinja as jinja_utils
 
 __all__ = [
     'get_pack_ref_from_metadata',
@@ -110,6 +112,14 @@ def validate_config_against_schema(config_schema, config_object, config_path,
         cleaned = util_schema.validate(instance=instance, schema=schema,
                                        cls=util_schema.CustomValidator, use_default=True,
                                        allow_default_none=True)
+        for key in cleaned:
+            if (jinja_utils.is_jinja_expression(value=cleaned.get(key)) and
+                    "decrypt_kv" in cleaned.get(key) and config_schema.get(key).get('secret')):
+                raise ValueValidationException('Values specified as "secret: True" in config '
+                                               'schema are automatically decrypted by default. Use '
+                                               'of "decrypt_kv" jinja filter is not allowed for '
+                                               'such values. Please check the specified values in '
+                                               'the config or the default values in the schema.')
     except jsonschema.ValidationError as e:
         attribute = getattr(e, 'path', [])
 

st2common/tests/unit/test_configs_registrar.py

@@ -36,6 +36,8 @@
 PACK_1_PATH = os.path.join(fixturesloader.get_fixtures_packs_base_path(), 'dummy_pack_1')
 PACK_6_PATH = os.path.join(fixturesloader.get_fixtures_packs_base_path(), 'dummy_pack_6')
 PACK_19_PATH = os.path.join(fixturesloader.get_fixtures_packs_base_path(), 'dummy_pack_19')
+PACK_11_PATH = os.path.join(fixturesloader.get_fixtures_packs_base_path(), 'dummy_pack_11')
+PACK_22_PATH = os.path.join(fixturesloader.get_fixtures_packs_base_path(), 'dummy_pack_22')
 
 
 class ConfigsRegistrarTestCase(CleanDbTestCase):
@@ -148,3 +150,63 @@ def test_register_all_configs_with_config_schema_validation_validation_failure_2
         self.assertRaisesRegexp(ValueError, expected_msg,
                                 registrar.register_from_packs,
                                 base_dirs=packs_base_paths)
+
+    def test_register_all_configs_with_config_schema_validation_validation_failure_3(self):
+        # This test checks for values containing "decrypt_kv" jinja filter in the config
+        # object where keys have "secret: True" set in the schema.
+
+        # Verify DB is empty
+        pack_dbs = Pack.get_all()
+        config_dbs = Config.get_all()
+
+        self.assertEqual(len(pack_dbs), 0)
+        self.assertEqual(len(config_dbs), 0)
+
+        registrar = ConfigsRegistrar(use_pack_cache=False, fail_on_failure=True,
+                                     validate_configs=True)
+        registrar._pack_loader.get_packs = mock.Mock()
+        registrar._pack_loader.get_packs.return_value = {'dummy_pack_11': PACK_11_PATH}
+
+        # Register ConfigSchema for pack
+        registrar._register_pack_db = mock.Mock()
+        registrar._register_pack(pack_name='dummy_pack_11', pack_dir=PACK_11_PATH)
+        packs_base_paths = content_utils.get_packs_base_paths()
+
+        expected_msg = ('Values specified as "secret: True" in config schema are automatically '
+                        'decrypted by default. Use of "decrypt_kv" jinja filter is not allowed '
+                        'for such values. Please check the specified values in the config or '
+                        'the default values in the schema.')
+
+        self.assertRaisesRegexp(ValueError, expected_msg,
+                                registrar.register_from_packs,
+                                base_dirs=packs_base_paths)
+
+    def test_register_all_configs_with_config_schema_validation_validation_failure_4(self):
+        # This test checks for default values containing "decrypt_kv" jinja filter for
+        # keys which have "secret: True" set.
+
+        # Verify DB is empty
+        pack_dbs = Pack.get_all()
+        config_dbs = Config.get_all()
+
+        self.assertEqual(len(pack_dbs), 0)
+        self.assertEqual(len(config_dbs), 0)
+
+        registrar = ConfigsRegistrar(use_pack_cache=False, fail_on_failure=True,
+                                     validate_configs=True)
+        registrar._pack_loader.get_packs = mock.Mock()
+        registrar._pack_loader.get_packs.return_value = {'dummy_pack_22': PACK_22_PATH}
+
+        # Register ConfigSchema for pack
+        registrar._register_pack_db = mock.Mock()
+        registrar._register_pack(pack_name='dummy_pack_22', pack_dir=PACK_22_PATH)
+        packs_base_paths = content_utils.get_packs_base_paths()
+
+        expected_msg = ('Values specified as "secret: True" in config schema are automatically '
+                        'decrypted by default. Use of "decrypt_kv" jinja filter is not allowed '
+                        'for such values. Please check the specified values in the config or '
+                        'the default values in the schema.')
+
+        self.assertRaisesRegexp(ValueError, expected_msg,
+                                registrar.register_from_packs,
+                                base_dirs=packs_base_paths)

st2tests/st2tests/fixtures/packs/configs/dummy_pack_11.yaml

@@ -0,0 +1,2 @@
+---
+  api_key: "{{st2kv.user.api_key | decrypt_kv}}"

st2tests/st2tests/fixtures/packs/configs/dummy_pack_22.yaml

@@ -0,0 +1,3 @@
+---
+  key_with_no_secret_and_no_default: "Any Value"
+

st2tests/st2tests/fixtures/packs/dummy_pack_11/config.schema.yaml

@@ -0,0 +1,5 @@
+---
+  api_key:
+    type: "string"
+    secret: true
+    required: true

st2tests/st2tests/fixtures/packs/dummy_pack_11/pack.yaml

@@ -0,0 +1,6 @@
+---
+name : dummy_pack_11
+description : dummy pack
+version : 0.1.0
+author : st2-dev
+email : info@stackstorm.com

st2tests/st2tests/fixtures/packs/dummy_pack_22/config.schema.yaml

@@ -0,0 +1,9 @@
+---
+  wrong_key_with_invalid_default_value:
+    type: "string"
+    secret: true
+    required: true
+    default: "{{st2kv.user.api_key | decrypt_kv}}"
+  key_with_no_secret_and_no_default:
+    type: "string"
+