exasol / integration-test-docker-environment

A docker-based environment for integration tests with the EXASOL DB.
https://exasol.github.io/integration-test-docker-environment/
MIT License
6 stars 2 forks source link

Fix CVE-2021-32559 #186

Closed Nicoretti closed 2 years ago

Nicoretti commented 2 years ago

The CVE-2021-32559 is caused by the transitive dependency pywin32, which is actually not needed by this package.

pywin32 is pulled in by the docker dependency, but is only needed on windows platforms (which ain't supported by this package).

By tigthening the dependency constrains of the docker dependency, we can get rid of the optional (not needed) transitive pywin32 dependency.

Fix #183

All Submissions:

tkilias commented 2 years ago

I don't understand what the fix actually does? Can you explain it?

Nicoretti commented 2 years ago

I don't understand what the fix actually does? Can you explain it?

Note: sadly poetry is not "feature" complete yet, in every way, otherwise we may could have expressed the platform dependency (linux, darwin) for our package and poetry would not even allow windows installs. -> Read a couple of open issues and PRs @poetry -> Work in Progress

Regarding the fix: It basically says the dependency "docker" is only needed on non win32 platforms. (pywin32 is a "feature flag" in the docker package)

Adding this constraint allows poetry while calculating the dependencies (.lock-file) to remove the transitive pywin32 dependency if done on a platform != win32. Because we do not support windows (explicitly stated in the README) this should be fine.

tkilias commented 2 years ago

Ah, it means only install the docker package on non-windows machines and this allows poetry to remove pywin32. Ok, is bit weird. You should add this as comment.

Nicoretti commented 2 years ago

Ah, it means only install the docker package on non-windows machines and this allows poetry to remove pywin32. Ok, is bit weird. You should add this as comment.

I'll do, and I agree it is a bit wired. It is mostly due to the fact that all other approaches still seem to have some issues. (when you search for --platform etc. in poetry issues/PRs).

I think the better solution would be if you could specify target platform constrains the package itself. (Could not find if or how that would work already)

Nicoretti commented 2 years ago

Looks good to me. Please just update the documentation.

documentation already states, that windows isn't supported. In case you meant adding a "note" in the dependency file :heavy_check_mark: . Also I added an additional health check to validate the platform we are running on.