Skip to content

Commit

Permalink
fix: respect NONE severity threshold
Browse files Browse the repository at this point in the history
When severity threshold is set to `none`, download should be allowed
regardless of the issues discovered.
  • Loading branch information
jacek-rzrz committed Nov 15, 2024
1 parent 1e2896c commit ff51614
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<Severity> vulnSeverityThreshold;

private final Severity licenseSeverityThreshold;
private final Optional<Severity> 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<Severity> vulnSeverityThreshold, Optional<Severity> licenseSeverityThreshold) {
this.vulnSeverityThreshold = vulnSeverityThreshold;
this.licenseSeverityThreshold = licenseSeverityThreshold;
}

public ValidationSettings withVulnSeverityThreshold(Severity threshold) {
public ValidationSettings withVulnSeverityThreshold(Optional<Severity> threshold) {
return new ValidationSettings(threshold, licenseSeverityThreshold);
}

public ValidationSettings withLicenseSeverityThreshold(Severity threshold) {
public ValidationSettings withLicenseSeverityThreshold(Optional<Severity> threshold) {
return new ValidationSettings(vulnSeverityThreshold, threshold);
}

public Severity getVulnSeverityThreshold() {
public Optional<Severity> getVulnSeverityThreshold() {
return vulnSeverityThreshold;
}

public Severity getLicenseSeverityThreshold() {
public Optional<Severity> 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<Severity> 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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Severity> 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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}


}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down

0 comments on commit ff51614

Please sign in to comment.