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

SG-20243: security fix #17

Merged
merged 6 commits into from
Jan 22, 2021
Merged

SG-20243: security fix #17

merged 6 commits into from
Jan 22, 2021

Conversation

jfboismenu
Copy link
Contributor

@jfboismenu jfboismenu commented Nov 17, 2020

This branch introduces a new script called update_python_packages.py that allows to repackage the zip file at the root of the repository. It uses pip and the requirements.txt to figure out what needs to be installed and uses pip freeze to dump the exact list of all python packages that were installed. This allows us to have the high level and low level view of what packages were installed and allows Github to notify us if we have unsecure dependencies.

Once all dependencies have been downloaded locally to a temporary folder, they are zipped up inside pkgs.zip.

@jfboismenu jfboismenu changed the title Security fix SG-20243: security fix Nov 17, 2020
# not expressly granted therein are reserved by Shotgun Software Inc.

requests==2.11.1
certifi==2020.6.20
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frozen requirements used to be the actual list of packages we install, since there wasn't any dependency, but now that file is generated by pip and requirements.txt is the actual list of stuff we want to install.

Copy link
Contributor

@pscadding pscadding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

)

# Quickly compute the number of requirements we have.
nb_dependencies = len([_ for _ in open("frozen_requirements.txt", "rt")])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this cause issues if we have comments in the frozen_requirements file? Though there shouldn't really, since it is auto generate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nm the assert further down would cause someone to look at it, and hopefully see the issue if there was.

@jfboismenu jfboismenu merged commit 6775354 into master Jan 22, 2021
@jfboismenu jfboismenu deleted the security-fix branch January 22, 2021 13:27
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.

2 participants