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

setup.py - remove insecure requests to https://pypi.org/pypi/python-appimage/json #37

Closed
wants to merge 1 commit into from

Conversation

manfred-kaiser
Copy link
Contributor

the request to https://pypi.org/pypi/python-appimage/json should be removed, because it breaks workflows, when running without an internet connection.

Another reason is, that you are using _create_unverified_context, which disables certificate validation.

This makes using you package insecure and prone to man in the middle attacks.

Determine the version number this way also breaks reproducible builds.

I have also created an issue for this: #36

@niess
Copy link
Owner

niess commented Jun 29, 2021

Hello @manfred-kaiser,

Thanks for checking this. I know little to nothing about security issues. But I believe you :P It sounds reasonable. I also agree with your other arguments: reproducibility, need of an internet connection.

I would be OK to remove the IP request to PyPI, I.e. the pypi versioning. However, I would keep the git hash generation in setup.py. The latter does not require any internet connection, only git.

With your proposed modification the git hash would be false because of cyclic deps. If version.py is under git then it cannot contain its own git hash since the latter depends on the content of version.py. This is the reason why I was generating version.py from setup.py, I.e. only when packaging.

Instead, would the following work for you:

  • Remove the version variable from the python_appimage.version module, i.e. keep only git_revision.
  • Generate version.py from setup,py but keeping only the git_revision related part.

@manfred-kaiser
Copy link
Contributor Author

I would suggest to use tags in git instead of saving the git meta data in you python application. With this approach, you can checkout a specific release from git.

Your approach only works, when you are using pypi.org. If you want to checkout a specific release, you have to download the version from pypi.org to get the git revison and then you can checkout this revision. If you have only access to the git repo, you will loose all version information :-(

There are some python modules, which are able to maintain the version for you: https://pypi.org/project/setuptools-git-versioning/

If you are interested, I can update the pull request to use one of the mentioned plugins.

@niess
Copy link
Owner

niess commented Jun 30, 2021

Hello @manfred-kaiser,

this setuptools-git-versioning looks interesting. But I am a bit confused how it would look like in practice. I am happy to see an example if you have?

Actually, I didn't really use the PyPI version so far. It is there because it must be in order to upload to PyPI. I use the git hash in order to refer to a specific state of the project. But indeed, from PyPI the git hash is not visible without downloading the wheel :(

But I am open to a more proper versioning if it does not imply to much extra maintenance :)

What was convenient to me with the current workflow is that I did not need to manually update the version. Basically, each push to Github is triggering a push to PyPI as well with an automatic "version" increment. Would setuptools-git-versioning allow a similar workflow? Maybe if it generates a version number that includes the git hash.

@manfred-kaiser
Copy link
Contributor Author

I will try to setup an example with setuptools-git-versioning.

Currently I want to remove the dependencies to remote resources. I want to use this tool on some devices which does not have a network connection.

@manfred-kaiser manfred-kaiser marked this pull request as draft July 6, 2021 15:41
@niess
Copy link
Owner

niess commented Jul 7, 2021

Great :)

I didn't think about this use case. It might be tricky to use in its current state without network. When packaging a manylinux AppImage docker is used. In addition some missing deps like patchelf are installed from GitHub. Though, once installed the network should not be required anymore.

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