#103: security warning for CVEs in file tool/edition/security #119
Conversation
MattesMrzik
changed the title
There was a problem hiding this comment.
@MattesMrzik Great that you found and reused UrlSecurityFile and you found the exact way to access and use it. 👍
I left some review comments to give some further direction and also you need to consider that we also have an existing security file for git.
As we do not have a commandlet for git we would either have to add GitCommandlet as global tool commandlet and also find a way to download the latest version of git without using ide-urls as cloning or pulling it already requires git or we keep that as a per-requisite of IDEasy but then have to find the right place in our code-base where to determine the currently installed git version and to do the security check. My perfect vision would be to have a GitCommandlet and even extend our git clone/pull method to check if git is already available and otherwise run the GitCommandlet to install it. However creating a GitCommandlet would cause a dependency to #113 so feel free to first ignore git so we can add it in a later PR.
Also we should find an additional tool version that we want to warn about (my suggestion would be gradle - see https://www.cvedetails.com/cve/CVE-2016-6199/) so we also have something to test this feature.
cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java
Outdated
Show resolved
Hide resolved
cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java
Outdated
Show resolved
Hide resolved
|
I've got a question: |
You are reaching the point why I said that the CVE database is a valuable source but we should not build a completely automated process but still need some manual process involved. |
cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java
Outdated
Show resolved
Hide resolved
ignore cves list, remove some analyzers, more test for version ranges like >, some cpe vendors and products to updaters
Extended the VersionRange to also model open boundaries that do not include the specified value. Also wrote tests for this class.
…implement-version-security-checks # Conflicts: # cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java # cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java
…e-with-open-intervals' into feature/devonfw#103-implement-version-security-checks # Conflicts: # cli/src/test/java/com/devonfw/tools/ide/version/VersionRangeTest.java
Pull Request Test Coverage Report for Build 8100325177Details
💛 - Coveralls |
if a single warning affects all versions, it is ignored
also SecurityRiskInteraction returns configured version and latest version when possible. conversion between cpe and ulr version more rebust by using map and inverse function where map fails. Added asciidoc
- changed pom.xml - getCpeEdition now has argument, since there is only a single UrlUpdater for multiple editions of a tool - some cleanup
added dependencyManagement to root pom.xml added owasp version property to root pom.xml renamed security artifact to ide-security
…ty-checks # Conflicts: # cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java # cli/src/test/resources/ide-projects/basic/_ide/urls/mvn/mvn/security.json
added missing answers param to newContext
fixed pom versions applied reformat
renamed retrievePath to getPath renamed addPath to setPath
added javadocs
removed warnings from security json
|
I've detected some more issues with the dependency checks which need to be addressed first. |
added missing CPE vendors/products
adjusted getCpeVendor and getCpeProduct to return the tool name instead of an empty string removed unused urlEdition param from getCpeEdition added workaround for intellij #1378 fixed NPE's (added checks for missing UrlUpdaters)
| /** | ||
| * User answer to install the latest of all safe versions. | ||
| */ | ||
| LATEST_SAFE, | ||
|
|
||
| /** | ||
| * User answer to use the latest safe version. | ||
| */ | ||
| SAFE_LATEST, |
There was a problem hiding this comment.
Sorry to say so but this is absolutely confusing. While there is LATEST_SAFE the JavaDoc of SAFE_LATEST says "use the latest safe version". The difference of these two options is totally unclear.
There was a problem hiding this comment.
I've renamed the constant to LATEST now instead and adjusted the javadoc.
There was a problem hiding this comment.
Just to give some update on this PR.
@jan-vcapgemini thank you so much for taking over and improving this PR.
It is already in a good state.
However, again we tend to create too big PRs and do too much in one go.
It would have been much simpler to first log a FAT WARNING if a unsecure version is used.
Here we already have complex user-interaction, etc.
Also I first would like to see a PR to ide-urls with the result of the tool so we can first discuss if the outcome really makes sense. And also without such security.json files the feature is not adding any value.
…ty-checks # Conflicts: # cli/pom.xml # cli/src/main/java/com/devonfw/tools/ide/common/SystemPath.java # cli/src/main/java/com/devonfw/tools/ide/tool/ToolCommandlet.java # cli/src/test/java/com/devonfw/tools/ide/context/AbstractIdeContextTest.java # documentation/LICENSE.adoc
added missing answers to IdeTestContext
renamed SAFE_LATEST to LATEST
Closes #103: Before installing a ToolCommandlet, warn the user if the selected version is listed in ulrs///security.json indicating that this version has a CVE and ask if the user wants to stay with the current or select a safe version.