372 Change yaml loader to safe loader (#373)

* Add safe loader with env interpolation and an arg to provide custom loader

* Add docs

* Update changelog

docs/main/changelog.rst

@@ -9,6 +9,10 @@ follows `Semantic versioning`_
 
 Development version
 -------------------
+- Add ``loader`` argument to the configuration provider ``Configuration.from_yaml(..., loader=...)`` to override the
+  default YAML loader.
+- Make security improvement: change default YAML loader to the custom ``yaml.SafeLoader`` with a support
+  of environment variables interpolation.
 - Fix a bug with asynchronous injections: async providers do not work with async dependencies.
   See issue: `#368 <https://github.com/ets-labs/python-dependency-injector/issues/368>`_.
   Thanks `@kolypto <https://github.com/kolypto>`_ for the bug report.

docs/providers/configuration.rst

@@ -21,6 +21,10 @@ Configuration provider
 
 It implements the principle "use first, define later".
 
+.. contents::
+   :local:
+   :backlinks: none
+
 Loading from an INI file
 ------------------------
 
@@ -57,9 +61,19 @@ where ``examples/providers/configuration/config.yml`` is:
 .. literalinclude:: ../../examples/providers/configuration/config.yml
    :language: ini
 
-:py:meth:`Configuration.from_yaml` method supports environment variables interpolation. Use
-``${ENV_NAME}`` format in the configuration file to substitute value of the environment
-variable ``ENV_NAME``.
+:py:meth:`Configuration.from_yaml` method uses custom version of ``yaml.SafeLoader``.
+
+The loader supports environment variables interpolation. Use ``${ENV_NAME}`` format
+in the configuration file to substitute value of the environment variable ``ENV_NAME``.
+
+You can also specify a YAML loader as an argument:
+
+.. code-block:: python
+
+   import yaml
+
+
+   container.config.from_yaml('config.yml', loader=yaml.UnsafeLoader)
 
 .. note::
 

src/dependency_injector/providers.pyi

@@ -20,6 +20,11 @@ from typing import (
     overload,
 )
 
+try:
+    import yaml
+except ImportError:
+    yaml = None
+
 from . import resources
 
 
@@ -150,7 +155,7 @@ class ConfigurationOption(Provider[Any]):
     def is_required(self) -> bool: ...
     def update(self, value: Any) -> None: ...
     def from_ini(self, filepath: Union[Path, str]) -> None: ...
-    def from_yaml(self, filepath: Union[Path, str]) -> None: ...
+    def from_yaml(self, filepath: Union[Path, str], loader: Optional[Any]=None) -> None: ...
     def from_dict(self, options: _Dict[str, Any]) -> None: ...
     def from_env(self, name: str, default: Optional[Any] = None) -> None: ...
 
@@ -171,7 +176,7 @@ class Configuration(Object[Any]):
     def reset_cache(self) -> None: ...
     def update(self, value: Any) -> None: ...
     def from_ini(self, filepath: Union[Path, str]) -> None: ...
-    def from_yaml(self, filepath: Union[Path, str]) -> None: ...
+    def from_yaml(self, filepath: Union[Path, str], loader: Optional[Any]=None) -> None: ...
     def from_dict(self, options: _Dict[str, Any]) -> None: ...
     def from_env(self, name: str, default: Optional[Any] = None) -> None: ...
 
@@ -373,3 +378,9 @@ def deepcopy(instance: Any, memo: Optional[_Dict[Any, Any]] = None): Any: ...
 
 
 def merge_dicts(dict1: _Dict[Any, Any], dict2: _Dict[Any, Any]) -> _Dict[Any, Any]: ...
+
+
+if yaml:
+    class YamlLoader(yaml.SafeLoader): ...
+else:
+    class YamlLoader: ...

src/dependency_injector/providers.pyx

@@ -89,6 +89,41 @@ else:
             return parser
 
 
+if yaml:
+    class YamlLoader(yaml.SafeLoader):
+        """Custom YAML loader.
+
+        Inherits ``yaml.SafeLoader`` and add environment variables interpolation.
+        """
+
+        tag = '!!str'
+        pattern = re.compile('.*?\${(\w+)}.*?')
+
+        @classmethod
+        def constructor_env_variables(cls, loader, node):
+            value = loader.construct_scalar(node)
+            match = cls.pattern.findall(value)
+            if match:
+                full_value = value
+                for group in match:
+                    full_value = full_value.replace(
+                        f'${{{group}}}', os.environ.get(group, group)
+                    )
+                return full_value
+            return value
+
+    # TODO: use SafeLoader without env interpolation by default in version 5.*
+    YamlLoader.add_implicit_resolver(YamlLoader.tag, YamlLoader.pattern, None)
+    YamlLoader.add_constructor(YamlLoader.tag, YamlLoader.constructor_env_variables)
+else:
+    class YamlLoader:
+        """Custom YAML loader.
+
+        Inherits ``yaml.SafeLoader`` and add environment variables interpolation.
+        """
+
+
+
 cdef int ASYNC_MODE_UNDEFINED = 0
 cdef int ASYNC_MODE_ENABLED = 1
 cdef int ASYNC_MODE_DISABLED = 2
@@ -1317,7 +1352,7 @@ cdef class ConfigurationOption(Provider):
             current_config = {}
         self.override(merge_dicts(current_config, config))
 
-    def from_yaml(self, filepath):
+    def from_yaml(self, filepath, loader=None):
         """Load configuration from the yaml file.
 
         Loaded configuration is merged recursively over existing configuration.
@@ -1325,6 +1360,9 @@ cdef class ConfigurationOption(Provider):
         :param filepath: Path to the configuration file.
         :type filepath: str
 
+        :param loader: YAML loader, :py:class:`YamlLoader` is used if not specified.
+        :type loader: ``yaml.Loader``
+
         :rtype: None
         """
         if yaml is None:
@@ -1334,9 +1372,13 @@ cdef class ConfigurationOption(Provider):
                 '"pip install dependency-injector[yaml]"'
             )
 
+
+        if loader is None:
+            loader = YamlLoader
+
         try:
             with open(filepath) as opened_file:
-                config = yaml.load(opened_file, yaml.Loader)
+                config = yaml.load(opened_file, loader)
         except IOError:
             return
 
@@ -1593,7 +1635,7 @@ cdef class Configuration(Object):
             current_config = {}
         self.override(merge_dicts(current_config, config))
 
-    def from_yaml(self, filepath):
+    def from_yaml(self, filepath, loader=None):
         """Load configuration from the yaml file.
 
         Loaded configuration is merged recursively over existing configuration.
@@ -1601,6 +1643,9 @@ cdef class Configuration(Object):
         :param filepath: Path to the configuration file.
         :type filepath: str
 
+        :param loader: YAML loader, :py:class:`YamlLoader` is used if not specified.
+        :type loader: ``yaml.Loader``
+
         :rtype: None
         """
         if yaml is None:
@@ -1610,9 +1655,12 @@ cdef class Configuration(Object):
                 '"pip install dependency-injector[yaml]"'
             )
 
+        if loader is None:
+            loader = YamlLoader
+
         try:
             with open(filepath) as opened_file:
-                config = yaml.load(opened_file, yaml.Loader)
+                config = yaml.load(opened_file, loader)
         except IOError:
             return
 

tests/unit/providers/test_configuration_py2_py3.py

@@ -9,6 +9,10 @@ import tempfile
 import unittest2 as unittest
 
 from dependency_injector import containers, providers, errors
+try:
+    import yaml
+except ImportError:
+    yaml = None
 
 
 class ConfigTests(unittest.TestCase):
@@ -581,6 +585,51 @@ class ConfigFromYamlWithEnvInterpolationTests(unittest.TestCase):
         self.assertEqual(self.config.section1(), {'value1': 'test-value'})
         self.assertEqual(self.config.section1.value1(), 'test-value')
 
+    @unittest.skipIf(sys.version_info[:2] == (3, 4), 'PyYAML does not support Python 3.4')
+    def test_option_env_variable_interpolation(self):
+        self.config.option.from_yaml(self.config_file)
+
+        self.assertEqual(
+            self.config.option(),
+            {
+                'section1': {
+                    'value1': 'test-value',
+                },
+            },
+        )
+        self.assertEqual(self.config.option.section1(), {'value1': 'test-value'})
+        self.assertEqual(self.config.option.section1.value1(), 'test-value')
+
+    @unittest.skipIf(sys.version_info[:2] == (3, 4), 'PyYAML does not support Python 3.4')
+    def test_env_variable_interpolation_custom_loader(self):
+        self.config.from_yaml(self.config_file, loader=yaml.UnsafeLoader)
+
+        self.assertEqual(
+            self.config(),
+            {
+                'section1': {
+                    'value1': 'test-value',
+                },
+            },
+        )
+        self.assertEqual(self.config.section1(), {'value1': 'test-value'})
+        self.assertEqual(self.config.section1.value1(), 'test-value')
+
+    @unittest.skipIf(sys.version_info[:2] == (3, 4), 'PyYAML does not support Python 3.4')
+    def test_option_env_variable_interpolation_custom_loader(self):
+        self.config.option.from_yaml(self.config_file, loader=yaml.UnsafeLoader)
+
+        self.assertEqual(
+            self.config.option(),
+            {
+                'section1': {
+                    'value1': 'test-value',
+                },
+            },
+        )
+        self.assertEqual(self.config.option.section1(), {'value1': 'test-value'})
+        self.assertEqual(self.config.option.section1.value1(), 'test-value')
+
 
 class ConfigFromDict(unittest.TestCase):