viur-framework / viur-core

The core component of ViUR, the Python framework for modern web development.
https://www.viur.dev
MIT License
13 stars 14 forks source link

chore: Use `pyproject.toml` as new SSOT packaging system #1224

Closed sveneberth closed 1 month ago

sveneberth commented 1 month ago

This PR includes several changes to the packaging system. The first is the complete replacement of setup.py and setup.cfg with pyproject.toml (which was already minimally existent). According to my rough research, these have become obsolete and are to be seen as legacy packaging with the introduction of pyproject.toml in PEP 517 and PEP 518. This was also illustrated in these posts:

Anyway, this is just a reference in case anyone wants to read up on it. With the help of the Python Packaging User Guide, I made one file (the pyproject.toml) out of three, if you count the Pipfile and requirements.txt, even five. I find that much easier to maintain only one single source of truth (SSOT).

The second major change is the removal of hashes and exact versions. We have already discussed this topic in detail in one of the past ViUR meetings. This makes the interaction with other python dependencies much easier, as the viur-core dependencies can now also be used in versions other than those specified in the requirements.txt. This means that security patches can be quickly integrated into the project even without a viur-core release. The risk of conflicts is also minimized. By choosing the appropriate operator (such as the compatibility operator ~= (which is already in use) or >=, minimum versions can still be enforced, even an exact==version if necessary). This is a breaking change.

Of course I have tested the build several times. By the way, the Pipfile is now only necessary for the build process. A build can be found on TestPyPi. Nevertheless, I would recommend first publishing an rc as a further test.


As a next step, we can extend our tox.ini in a follow-up PR and try to define the dependencies and commands for the unit tests there, as it is solved in Jinja, for example. But for now, that should be enough.

sveneberth commented 1 month ago

One of the reasons for this PR is that I have several vulnerabilities warnings through the viur-cli again when deploying. And I can't upgrade these versions as long as the viur-core wants exactly these versions.

viur-cli stdout extract

``` Checking Pipfile.lock packages for vulnerabilities... 3 vulnerabilities found. 71600: gunicorn 21.2.0 open to vulnerability 71600 (<22.0.0). More info: https://data.safetycli.com/v/71600/742 Gunicorn fails to properly validate Transfer-Encoding headers, leading to HTTP Request Smuggling (HRS) vulnerabilities. By crafting requests with conflicting Transfer-Encoding headers, attackers can bypass security restrictions and access restricted endpoints. This issue is due to Gunicorn's handling of Transfer-Encoding headers, where it incorrectly processes requests with multiple, conflicting Transfer-Encoding headers, treating them as chunked regardless of the final encoding specified. This vulnerability allows for a range of attacks including cache poisoning, session manipulation, and data exposure. 70612: jinja2 3.1.4 open to vulnerability 70612 (>=0). More info: https://data.safetycli.com/v/70612/742 In Jinja2, the from_string function is prone to Server Side Template Injection (SSTI) where it takes the "source" parameter as a template object, renders it, and then returns it. The attacker can exploit it with {{INJECTION COMMANDS}} in a URI. NOTE: The maintainer and multiple third parties believe that this vulnerability isn't valid because users shouldn't use untrusted templates without sandboxing. 71608: urllib3 1.26.18 open to vulnerability 71608 (<=1.26.18). More info: https://data.safetycli.com/v/71608/742 Urllib3's ProxyManager ensures that the Proxy-Authorization header is correctly directed only to configured proxies. However, when HTTP requests bypass urllib3's proxy support, there's a risk of inadvertently setting the Proxy-Authorization header, which remains ineffective without a forwarding or tunneling proxy. Urllib3 does not recognize this header as carrying authentication data, failing to remove it during cross-origin redirects. While this scenario is uncommon and poses low risk to most users, urllib3 now proactively removes the Proxy-Authorization header during cross-origin redirects as a precautionary measure. Users are advised to utilize urllib3's proxy support or disable automatic redirects to handle the Proxy-Authorization header securely. Despite these precautions, urllib3 defaults to stripping the header to safeguard users who may inadvertently misconfigure requests. ```

And also that I don't want to work on a yanked version that has verbose logging enabled. See #1222

CRC-Mismatch commented 1 month ago

Dropping in out of nowhere, but if I may, using Poetry, you could also run a command before anything else to export a requirements.ini in CI/CD workflows to ease the migration process

sveneberth commented 1 month ago

Dropping in out of nowhere, but if I may, using Poetry, you could also run a command before anything else to export a requirements.ini in CI/CD workflows to ease the migration process

Hello @CRC-Mismatch, thank you for your input. I've heard of poetry before, but I haven't really looked at it. It looks quite interesting. Within our projects we use Pipenv as a dependency management and environment tool, which is currently working well for us. With pipfile2req we can also generate a requirements.txt for our glcoud deployment. Here in viur-core it's still a bit of a mess with dependencies for the actual package and the unittests and docs. In the future, I will break this down step by step and evaluate which tool is most suitable for us and whether poetry might be the better choice here. If you have a more specific suggestion or if I have misunderstood you, please let us know, preferably with an example or a project for reference that uses it.

sveneberth commented 1 month ago

Do you think this change is necessary for 3.6, or shall we integrate this starting with 3.7?

I would implement it asap. And since I think it could take a while with 3.7, I would do it in 3.6 (SemVer didn't work here again anyway :neutral_face:). Hence the PR for the main.