From ff5161402ead392ee7c68ca6b4595a3ddeda82c8 Mon Sep 17 00:00:00 2001 From: Jacek Rzeniewicz Date: Fri, 15 Nov 2024 17:26:45 +0100 Subject: [PATCH] fix: respect NONE severity threshold When severity threshold is set to `none`, download should be allowed regardless of the issues discovered. --- .../artifactory/model/ValidationSettings.java | 43 ++++++++++++++----- .../artifactory/scanner/PackageValidator.java | 11 ++++- .../model/ValidationSettingsTest.java | 34 +++++++++++++++ .../scanner/PackageValidatorTest.java | 42 +++++++++++++----- 4 files changed, 106 insertions(+), 24 deletions(-) create mode 100644 core/src/test/java/io/snyk/plugins/artifactory/model/ValidationSettingsTest.java diff --git a/core/src/main/java/io/snyk/plugins/artifactory/model/ValidationSettings.java b/core/src/main/java/io/snyk/plugins/artifactory/model/ValidationSettings.java index 03b3001..1417917 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/model/ValidationSettings.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/model/ValidationSettings.java @@ -3,43 +3,64 @@ import io.snyk.plugins.artifactory.configuration.ConfigurationModule; import io.snyk.sdk.model.Severity; +import java.util.Optional; + import static io.snyk.plugins.artifactory.configuration.PluginConfiguration.SCANNER_LICENSE_THRESHOLD; import static io.snyk.plugins.artifactory.configuration.PluginConfiguration.SCANNER_VULNERABILITY_THRESHOLD; public class ValidationSettings { - private final Severity vulnSeverityThreshold; + private final Optional vulnSeverityThreshold; - private final Severity licenseSeverityThreshold; + private final Optional licenseSeverityThreshold; public ValidationSettings() { - this(Severity.HIGH, Severity.HIGH); + this(Optional.of(Severity.HIGH), Optional.of(Severity.HIGH)); } - private ValidationSettings(Severity vulnSeverityThreshold, Severity licenseSeverityThreshold) { + private ValidationSettings(Optional vulnSeverityThreshold, Optional licenseSeverityThreshold) { this.vulnSeverityThreshold = vulnSeverityThreshold; this.licenseSeverityThreshold = licenseSeverityThreshold; } - public ValidationSettings withVulnSeverityThreshold(Severity threshold) { + public ValidationSettings withVulnSeverityThreshold(Optional threshold) { return new ValidationSettings(threshold, licenseSeverityThreshold); } - public ValidationSettings withLicenseSeverityThreshold(Severity threshold) { + public ValidationSettings withLicenseSeverityThreshold(Optional threshold) { return new ValidationSettings(vulnSeverityThreshold, threshold); } - public Severity getVulnSeverityThreshold() { + public Optional getVulnSeverityThreshold() { return vulnSeverityThreshold; } - public Severity getLicenseSeverityThreshold() { + public Optional getLicenseSeverityThreshold() { return licenseSeverityThreshold; } public static ValidationSettings from(ConfigurationModule config) { - return new ValidationSettings() - .withVulnSeverityThreshold(Severity.of(config.getPropertyOrDefault(SCANNER_VULNERABILITY_THRESHOLD))) - .withLicenseSeverityThreshold(Severity.of(config.getPropertyOrDefault(SCANNER_LICENSE_THRESHOLD))); + return from( + config.getPropertyOrDefault(SCANNER_VULNERABILITY_THRESHOLD), + config.getPropertyOrDefault(SCANNER_LICENSE_THRESHOLD) + ); + } + + public static ValidationSettings from(String vulnThreshold, String licenseThreshold) { + return new ValidationSettings( + parseSeverity(vulnThreshold), + parseSeverity(licenseThreshold) + ); + } + + private static Optional parseSeverity(String severityStr) { + if ("none".equalsIgnoreCase(severityStr)) { + return Optional.empty(); + } + Severity severity = Severity.of(severityStr); + if (severity == null) { + throw new IllegalArgumentException("Invalid severity threshold: " + severityStr); + } + return Optional.of(severity); } } diff --git a/core/src/main/java/io/snyk/plugins/artifactory/scanner/PackageValidator.java b/core/src/main/java/io/snyk/plugins/artifactory/scanner/PackageValidator.java index 0eb43eb..772cbd3 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/scanner/PackageValidator.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/scanner/PackageValidator.java @@ -8,6 +8,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import java.util.Optional; + import static java.lang.String.format; public class PackageValidator { @@ -45,8 +47,13 @@ private void validateLicenseIssues(MonitoredArtifact artifact) { ); } - private void validateIssues(IssueSummary summary, Severity threshold, boolean ignoreIssues, String issueType, MonitoredArtifact artifact) { - int countAboveThreshold = summary.getCountAtOrAbove(threshold); + private void validateIssues(IssueSummary summary, Optional threshold, boolean ignoreIssues, String issueType, MonitoredArtifact artifact) { + if(threshold.isEmpty()) { + LOG.debug("No severity threshold set for {}", issueType); + return; + } + + int countAboveThreshold = summary.getCountAtOrAbove(threshold.get()); if (countAboveThreshold == 0) { LOG.debug("No {} with severity {} or higher: {}", issueType, threshold, artifact.getPath()); return; diff --git a/core/src/test/java/io/snyk/plugins/artifactory/model/ValidationSettingsTest.java b/core/src/test/java/io/snyk/plugins/artifactory/model/ValidationSettingsTest.java new file mode 100644 index 0000000..feb6631 --- /dev/null +++ b/core/src/test/java/io/snyk/plugins/artifactory/model/ValidationSettingsTest.java @@ -0,0 +1,34 @@ +package io.snyk.plugins.artifactory.model; + +import io.snyk.sdk.model.Severity; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; + +class ValidationSettingsTest { + + @Test + void from_whenValidThresholds() { + ValidationSettings settings = ValidationSettings.from("high", "low"); + + assertThat(settings.getVulnSeverityThreshold()).contains(Severity.HIGH); + assertThat(settings.getLicenseSeverityThreshold()).contains(Severity.LOW); + } + + @Test + void from_whenInvalidThresholds() { + assertThatThrownBy(() -> ValidationSettings.from("danger", "low")) + .hasMessageContaining("danger"); + } + + @Test + void from_whenNoThresholds() { + ValidationSettings settings = ValidationSettings.from("none", "none"); + + assertThat(settings.getVulnSeverityThreshold()).isEmpty(); + assertThat(settings.getLicenseSeverityThreshold()).isEmpty(); + } + + +} diff --git a/core/src/test/java/io/snyk/plugins/artifactory/scanner/PackageValidatorTest.java b/core/src/test/java/io/snyk/plugins/artifactory/scanner/PackageValidatorTest.java index 7266ebd..4ea7029 100644 --- a/core/src/test/java/io/snyk/plugins/artifactory/scanner/PackageValidatorTest.java +++ b/core/src/test/java/io/snyk/plugins/artifactory/scanner/PackageValidatorTest.java @@ -6,6 +6,7 @@ import org.junit.jupiter.api.Test; import java.net.URI; +import java.util.Optional; import java.util.stream.Stream; import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; @@ -16,8 +17,8 @@ class PackageValidatorTest { @Test void validate_severityBelowThreshold_allowed() { ValidationSettings settings = new ValidationSettings() - .withVulnSeverityThreshold(Severity.MEDIUM) - .withLicenseSeverityThreshold(Severity.CRITICAL); + .withVulnSeverityThreshold(Optional.of(Severity.MEDIUM)) + .withLicenseSeverityThreshold(Optional.of(Severity.CRITICAL)); PackageValidator validator = new PackageValidator(settings); MonitoredArtifact artifact = new MonitoredArtifact("", new TestResult( @@ -34,8 +35,8 @@ void validate_severityBelowThreshold_allowed() { @Test void validate_vulnIssueAboveThreshold_forbidden() { ValidationSettings settings = new ValidationSettings() - .withVulnSeverityThreshold(Severity.HIGH) - .withLicenseSeverityThreshold(Severity.LOW); + .withVulnSeverityThreshold(Optional.of(Severity.HIGH)) + .withLicenseSeverityThreshold(Optional.of(Severity.LOW)); PackageValidator validator = new PackageValidator(settings); MonitoredArtifact artifact = new MonitoredArtifact("", new TestResult( @@ -52,8 +53,8 @@ void validate_vulnIssueAboveThreshold_forbidden() { @Test void validate_vulnIssuesIgnored_allowed() { ValidationSettings settings = new ValidationSettings() - .withVulnSeverityThreshold(Severity.HIGH) - .withLicenseSeverityThreshold(Severity.LOW); + .withVulnSeverityThreshold(Optional.of(Severity.HIGH)) + .withLicenseSeverityThreshold(Optional.of(Severity.LOW)); PackageValidator validator = new PackageValidator(settings); MonitoredArtifact artifact = new MonitoredArtifact("", new TestResult( @@ -70,8 +71,8 @@ void validate_vulnIssuesIgnored_allowed() { @Test void validate_licenseIssueAboveThreshold_forbidden() { ValidationSettings settings = new ValidationSettings() - .withVulnSeverityThreshold(Severity.LOW) - .withLicenseSeverityThreshold(Severity.MEDIUM); + .withVulnSeverityThreshold(Optional.of(Severity.LOW)) + .withLicenseSeverityThreshold(Optional.of(Severity.MEDIUM)); PackageValidator validator = new PackageValidator(settings); MonitoredArtifact artifact = new MonitoredArtifact("", new TestResult( @@ -85,11 +86,30 @@ void validate_licenseIssueAboveThreshold_forbidden() { assertThatThrownBy(() -> validator.validate(artifact)).isExactlyInstanceOf(CancelException.class); } + + @Test + void validate_thresholdNone_allowed() { + ValidationSettings settings = new ValidationSettings() + .withVulnSeverityThreshold(Optional.empty()) + .withLicenseSeverityThreshold(Optional.empty()); + PackageValidator validator = new PackageValidator(settings); + MonitoredArtifact artifact = new MonitoredArtifact("", + new TestResult( + IssueSummary.from(Stream.of(Severity.CRITICAL)), + IssueSummary.from(Stream.of(Severity.CRITICAL)), + URI.create("https://snyk.io/package/version") + ), + new Ignores() + ); + + assertThatCode(() -> validator.validate(artifact)).doesNotThrowAnyException(); + } + @Test void validate_licenseIssuesIgnored_allowed() { ValidationSettings settings = new ValidationSettings() - .withVulnSeverityThreshold(Severity.LOW) - .withLicenseSeverityThreshold(Severity.MEDIUM); + .withVulnSeverityThreshold(Optional.of(Severity.LOW)) + .withLicenseSeverityThreshold(Optional.of(Severity.MEDIUM)); PackageValidator validator = new PackageValidator(settings); MonitoredArtifact artifact = new MonitoredArtifact("", new TestResult( @@ -106,7 +126,7 @@ void validate_licenseIssuesIgnored_allowed() { @Test void validate_includesSnykDetailsUrlInCancelException() { ValidationSettings settings = new ValidationSettings() - .withVulnSeverityThreshold(Severity.LOW); + .withVulnSeverityThreshold(Optional.of(Severity.LOW)); PackageValidator validator = new PackageValidator(settings); MonitoredArtifact artifact = new MonitoredArtifact("", new TestResult(