[SECRES-2397] Enforce pip and npm version compatibility #13
Conversation
ikretz
changed the title
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| """ | ||
| def get_executable() -> str: | ||
| if (venv := os.environ.get("VIRTUAL_ENV")): | ||
| return os.path.join(venv, "bin/python") | ||
| else: | ||
| return sys.executable | ||
|
|
||
| def get_pip_version(executable: str) -> Version: |
There was a problem hiding this comment.
I feel this can be a method check_version() of the abstract obj that returns bool instead of raising an error while getting the executable.
I think that such method can be reused in other parts of the code the future, but I don't have any strong opinion.
There was a problem hiding this comment.
Tbh what you're describing was my original vision for this change. In the end, I decided to go with this error-based approach instead because I think it will overall lead to safer and more maintainable code.
In the check_version() style, to figure out whether the executable version is supported, you have to allow PackageManagerCommand instances for unsupported versions to exist. But these latter instances have access to all of the other PackageManagerCommand methods that should only be executed when we have a supported version. It means other parts of the code have to be much more careful about what kind of PackageManagerCommand instance it has, which probably means calling check_version() before proceeding with anything.
On the other hand, in the error-based approach, if you can instantiate the PackageManagerCommand, you know it's safe to use all of its methods.
The check_version() style has advantages, though, and if down the road we find ourselves needing to perform version checks in other parts of the code, we can change it.
This PR changes the behavior of the firewall so that it refuses to run if an attempt is made to use it with an unsupported version of
pipornpm. This is in keeping with the firewall's primary goal of blocking 100% of vulnerable or malicious package installations within the purview of its datasets.This is done by adding a custom exception,
UnsupportedVersionError, which subclasses ofPackageManagerCommandare intended to use to signal an incompatible version at initialization. The firewall handles these errors gracefully, logging the error and exiting normally. It will be very simple to change the code later on if we decide to let the user decide whether to run with an incompatible version in spite of this fact.The PR also adds additional warning and info logs in the
PipCommandandNpmCommandmodules.