Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicitly use UnsafeLoader to load config/state #29

Merged
merged 1 commit into from
Aug 24, 2021

Conversation

ajdecon
Copy link
Contributor

@ajdecon ajdecon commented Aug 24, 2021

PR #22 merged an upgrade to PyYAML which broke our existing use of
yaml.load for configuration and state files.

Unfortunately, we can't use safe_load here because we make use of
defaultdict datastructures that don't work with this method:

yaml.constructor.ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/object/apply:collections.defaultdict'

So this PR explicitly uses the UnsafeLoader to load the config and state
files.

This should be fine, as we're not accepting any arbitrary data
from untrusted users for this configuration. Anyone who is using this
tool has Docker access, so should be assumed to be an administrator
anyway.

PR NVIDIA#22 merged an upgrade to PyYAML which broke our existing use of
yaml.load for configuration and state files.

Unfortunately, we can't use safe_load here because we make use of
`defaultdict` datastructures that don't work with this method:

```
yaml.constructor.ConstructorError: could not determine a constructor for the tag 'tag:yaml.org,2002:python/object/apply:collections.defaultdict'
```

So this PR explicitly uses the UnsafeLoader to load the config and state
files.

This should be fine, as we're not accepting any arbitrary data
from untrusted users for this configuration. Anyone who is using this
tool has Docker access, so should be assumed to be an administrator
anyway.
@ryanolson ryanolson merged commit 2b7c58e into NVIDIA:master Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants