Use yaml.safe_load instead of yaml.load

[julianolf]

As explained on issue #453 the use of the yaml.load function allows the execution of arbitrary python commands what is quite dangerous and discouraged, the problem can be solved by simply using the function safe_load instead.

In additional, two tests were added to ensure that safe_load is working as expected by properly loading valid yaml files and avoiding the execution of unstrusted commands.

Solves issue #453.

[BoboTiG]

Hello @julianolf, thank you very much for addressing this issue.

However I would prefer keeping test files using pytest format instead of unittest. I would rewrote to something like that:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

import os

from watchdog import watchmedo

import pytest
import yaml


def test_load_config_valid(tmpdir):
    """Verifies the load of a valid yaml file"""

    yaml_file = os.path.join(tmpdir, 'config_file.yaml')
    with open(yaml_file, "w") as f:
        f.write('one: value\ntwo:\n- value1\n- value2\n')

    config = watchmedo.load_config(yaml_file)
    assert isinstance(config, dict)
    assert 'one' in config
    assert 'two' in config
    assert isinstance(config['two'], list)
    assert config['one'] == 'value'
    assert config['two'] == ['value1', 'value2']


def test_load_config_invalid(tmpdir):
    """Verifies if safe load avoid the execution
    of untrusted code inside yaml files"""

    critical_dir = os.path.join(tmpdir, 'critical')
    yaml_file = os.path.join(tmpdir, 'tricks_file.yaml')
    with open(yaml_file, "w") as f:
        content = (
            'one: value\n'
            'run: !!python/object/apply:os.system ["mkdir {}"]\n'
        ).format(critical_dir)
        f.write(content)

    with pytest.raises(yaml.constructor.ConstructorError):
        watchmedo.load_config(yaml_file)

    assert not os.path.exists(critical_dir)

Do you mind using this version or a similar one using pytest?

[julianolf]

Do you mind using this version or a similar one using pytest?

Hi @BoboTiG, not at all, I'm going to switch to use your version of the tests and send a new commit. I was actually concerned on how write those tests as I saw most of the existing tests was using pytest with one or two exceptions. I should've asked first. Anyway, thanks for the feedback. :)

[BoboTiG]

Thanks 💪