PyYAML-5.1 breaks !secret tag parsing

[mvn23]

After an update with pip3 install --upgrade appdaemon, AD would not start anymore. The systemd journal shows the following:

Mar 23 19:50:45 shodan appdaemon[18236]: /opt/appdaemon/lib/python3.5/site-packages/appdaemon/admain.py:184: YAMLLoadWarning: calling yaml.load() without Loader=... is deprecated, as the default Loader is unsaf
Mar 23 19:50:45 shodan appdaemon[18236]:   config = yaml.load(config_file_contents)
Mar 23 19:50:45 shodan appdaemon[18236]: ERROR Error loading configuration
Mar 23 19:50:45 shodan appdaemon[18236]: ERROR parser says
Mar 23 19:50:45 shodan appdaemon[18236]: ERROR   in "<unicode string>", line 8, column 14:
Mar 23 19:50:45 shodan appdaemon[18236]:           token: !secret ha_token
Mar 23 19:50:45 shodan appdaemon[18236]:                  ^
Mar 23 19:50:45 shodan appdaemon[18236]: ERROR could not determine a constructor for the tag '!secret'

Downgrading PyYAML to 3.13 solves the issue.
This could be an upstream issue, since it looks like there are some problems with 5.1 which may be related and possibly fixed in 5.2: yaml/pyyaml#265

[acockburn]

Thanks for the info - I'll look into pinning the version until they figure it out.

[Luke15153]

This issue occurs also with the latest addon v2.0.1.
With v1.7 everything was fine, v2.0.1 won't start any more.

[acockburn]

@Luke15153 You need to report that separately as an issue with the addon, as I have no control of the environment there.

[frenck]

@acockburn The add-on does only require AppDaemon and nothing else. See:

This add-on does only install AppDaemon:

https://github.com/hassio-addons/addon-appdaemon3/blob/master/appdaemon/requirements.txt

Dependencies of AppDaemon are not locked in by the add-on, but are defined in:

https://github.com/home-assistant/appdaemon/blob/dev/setup.py#L18

So could you please add some proper version locking instead of pointing people to the add-on?

👍

[frenck]

PS: I could do a version pin on the add-on end to help people, but IMHO, it should be fixed at the root.

[acockburn]

I agree and ultimately am planning to fix it in the next release which is planned to be a version bump to 4.0 with a lot of changes in it. This may be some time yet as it needs some fixes and I am out of time to work on it, which means the addon will be broken until I can get to it. The only thing I can do in the short term is release a fix on the 3.0.x version and hope it works OK as I don't have a test environment setup for 3.x any more.

[frenck]

@acockburn Check, thanks for explaining. In that case, I'll lock the PyYAML in the add-on for now to 3.13, that will take the heat of the issue.

[acockburn]

No worries - I am making the change now

[acockburn]

Just about to cut the release and I'll make a PR as usual

[frenck]

No need to do the PR
That is automated now 😉

Thanks @acockburn 👍

[acockburn]

Ahh, very cool :)

Well I pushed the fix to pypi so we shuld be good to go.

[mvn23]

Works like a charm. Thanks!

[acockburn]

@mvn23 - could you do me a favor please? I have discovered that version 3.13 of PyYaml has a serious flaw in it and I need to upgrade to at least 4.2b1 to fix it. However, I was unable to reproduce your original problem. Could you confirm that upgrading to 4.2b1 doesn't cause the original issue to return? If you can do that for me I'll release another version to upgrade PyYaml to that level., Thanks!

[mvn23]

Unfortunately, everything after pyyaml==3.13 breaks the tag parsing. I assume that by 'serious flaw' you mean CVE-2017-18342, in which case the fix for this is exactly what is causing the issue.
I made a fix on the master branch which uses yaml.SafeLoader through the Loader= parameter as per the pyyaml wiki. This works with pyyaml==5.1 as well.
PR will follow in a few minutes.

edit: The same fix could be implemented on dev as well.

[acockburn]

Excellent thanks! I'll merge on master and release a build then I can just merge the same commits into dev.

[acockburn]

AppDaemon 3.0.4 just released with your safeloader fix and pyyaml pinned to 5.1