From 1f6fedf002fb9591964d7f55acfc862442bd9a32 Mon Sep 17 00:00:00 2001 From: Jacek Rzeniewicz Date: Mon, 11 Nov 2024 09:17:44 +0000 Subject: [PATCH] feat: add continuous testing mode Introducing a new lifecycle method `beforeRemoteDownload` which applies when a package is being fetched from external repositories instead of on every download. Adding a new configuration property `snyk.scanner.test.continuously` which allowes users to switch between applying the plugin on every download (continuous mode) or just once during fetch from remote (non-continuous mode). --- README.md | 1 + .../artifactory/snykSecurityPlugin.groovy | 11 ++++ .../artifactory/snykSecurityPlugin.properties | 12 +++- .../snyk/plugins/artifactory/SnykPlugin.java | 42 +++++++++++-- .../configuration/PluginConfiguration.java | 1 + .../plugins/artifactory/model/TestResult.java | 6 +- .../artifactory/scanner/ArtifactCache.java | 14 ++--- .../artifactory/scanner/ArtifactResolver.java | 12 ++++ .../scanner/ReadOnlyArtifactResolver.java | 15 +++++ .../artifactory/scanner/ScannerModule.java | 56 ++++++++++++----- .../model/MonitoredArtifactTest.java | 62 +++++++++++-------- .../scanner/ArtifactCacheTest.java | 32 +++++----- .../scanner/ScannerModuleTest.java | 14 ++--- 13 files changed, 196 insertions(+), 82 deletions(-) create mode 100644 core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactResolver.java create mode 100644 core/src/main/java/io/snyk/plugins/artifactory/scanner/ReadOnlyArtifactResolver.java diff --git a/README.md b/README.md index de11c9b..9480bbf 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ After making changes to the plugin, repeat `mvn install` and extract the jar fil ```shell unzip -p distribution/target/artifactory-snyk-security-plugin-LOCAL-SNAPSHOT.zip plugins/lib/artifactory-snyk-security-core.jar > distribution/docker/etc/artifactory/plugins/lib/artifactory-snyk-security-core.jar +unzip -p distribution/target/artifactory-snyk-security-plugin-LOCAL-SNAPSHOT.zip plugins/snykSecurityPlugin.groovy > distribution/docker/etc/artifactory/plugins/snykSecurityPlugin.groovy ``` ## Inspecting plugin logs diff --git a/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.groovy b/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.groovy index b69f7ce..76ad7a5 100644 --- a/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.groovy +++ b/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.groovy @@ -25,6 +25,16 @@ executions { } download { + + afterRemoteDownload { Request request, RepoPath repoPath -> + try { + snykPlugin.handleAfterRemoteDownloadEvent(repoPath) + } catch (Exception e) { + log.error("An exception occurred during afterRemoteDownload, re-throwing it for Artifactory to handle. Message was: ${e.message}") + throw e + } + } + beforeDownload { Request request, RepoPath repoPath -> try { snykPlugin.handleBeforeDownloadEvent(repoPath) @@ -33,6 +43,7 @@ download { throw e } } + } storage { diff --git a/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.properties b/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.properties index 3e53056..481dcdd 100644 --- a/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.properties +++ b/core/src/main/groovy/io/snyk/plugins/artifactory/snykSecurityPlugin.properties @@ -44,13 +44,21 @@ snyk.api.organization= # Scanner Configuration # ===================== -# Scan result expiry. When the most recent scan was made within this time frame, +# Decides whether the plugin should periodically refresh vulnerability data from Snyk +# or filter access according to results obtained while the package was first requested. +# Without the continuous mode, new vulnerabilities aren't reported for a package that has already been +# allowed through the gatekeeper. +# Accepts: "true", "false" +# Default: "false" +#snyk.scanner.test.continuously=false + +# Scan result expiry (continuous mode only). When the most recent scan was made within this time frame, # filtering respects the previous result. Beyond that time, a new Snyk Test request is made. # When this property is set to 0, the plugin triggers a test each time an artifact is accessed. # Default: 168 (1 week) #snyk.scanner.frequency.hours=168 -# How much to extend the scan result expiry when a Snyk Test request fails. +# How much to extend the scan result expiry when a Snyk Test request fails (continuous mode only). # In case there is a Snyk request error when the next test is due, # this parameter allows the plugin to use the previous test result when deciding whether to block access. # Beyond this extended deadline, the result of filtering will depend on the snyk.scanner.block-on-api-failure param. diff --git a/core/src/main/java/io/snyk/plugins/artifactory/SnykPlugin.java b/core/src/main/java/io/snyk/plugins/artifactory/SnykPlugin.java index 91eb3ab..39a8f0a 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/SnykPlugin.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/SnykPlugin.java @@ -88,23 +88,42 @@ public void handleAfterPropertyCreateEvent(User user, ItemInfo itemInfo, String } /** - * Scans an artifact for issues (vulnerability or license). + * Invoked once when an artifact is first fetched from an external repository. + * Runs Snyk test and persists the result in properties. + *

+ * Extension point: {@code download.afterRemoteDownload}. + */ + public void handleAfterRemoteDownloadEvent(RepoPath repoPath) { + LOG.debug("Handle 'afterRemoteDownload' event for: {}", repoPath); + + try { + scannerModule.testArtifact(repoPath); + } catch (CannotScanException e) { + LOG.debug("Artifact cannot be scanned. {} {}", e.getMessage(), repoPath); + } catch(SnykAPIFailureException e) { + String causeMessage = getCauseMessage(e); + String message = format("Snyk test failed. %s %s", causeMessage, repoPath); + LOG.error(message); + } + } + + /** + * Filters access based on Snyk properties stored on the artifact. + * When in continuous mode, may run an extra Snyk test to refresh the results. *

* Extension point: {@code download.beforeDownload}. */ public void handleBeforeDownloadEvent(RepoPath repoPath) { LOG.debug("Handle 'beforeDownload' event for: {}", repoPath); + try { - scannerModule.scanArtifact(repoPath); + scannerModule.filterAccess(repoPath); } catch (CannotScanException e) { LOG.debug("Artifact cannot be scanned. {} {}", e.getMessage(), repoPath); } catch (SnykAPIFailureException e) { final String blockOnApiFailurePropertyKey = SCANNER_BLOCK_ON_API_FAILURE.propertyKey(); final String blockOnApiFailure = configurationModule.getPropertyOrDefault(SCANNER_BLOCK_ON_API_FAILURE); - final String causeMessage = Optional.ofNullable(e.getCause()) - .map(Throwable::getMessage) - .map(m -> e.getMessage() + " " + m) - .orElseGet(e::getMessage); + final String causeMessage = getCauseMessage(e); String message = format("Artifact scan failed due to an API error on Snyk's side. %s %s", causeMessage, repoPath); LOG.debug(message); @@ -115,6 +134,13 @@ public void handleBeforeDownloadEvent(RepoPath repoPath) { } } + private String getCauseMessage(Throwable e) { + return Optional.ofNullable(e.getCause()) + .map(Throwable::getMessage) + .map(m -> e.getMessage() + " " + m) + .orElseGet(e::getMessage); + } + private void validateConfiguration() { try { configurationModule.validate(); @@ -131,6 +157,10 @@ private void validateConfiguration() { .forEach(LOG::debug); } + private boolean shouldTestContinuously() { + return configurationModule.getPropertyOrDefault(TEST_CONTINUOUSLY).equals("true"); + } + private SnykClient createSnykClient(@Nonnull ConfigurationModule configurationModule, String pluginVersion) throws Exception { final String token = configurationModule.getPropertyOrDefault(API_TOKEN); String baseUrl = configurationModule.getPropertyOrDefault(API_URL); diff --git a/core/src/main/java/io/snyk/plugins/artifactory/configuration/PluginConfiguration.java b/core/src/main/java/io/snyk/plugins/artifactory/configuration/PluginConfiguration.java index 6978706..2bcb527 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/configuration/PluginConfiguration.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/configuration/PluginConfiguration.java @@ -19,6 +19,7 @@ public enum PluginConfiguration implements Configuration { SCANNER_PACKAGE_TYPE_MAVEN("snyk.scanner.packageType.maven", "true"), SCANNER_PACKAGE_TYPE_NPM("snyk.scanner.packageType.npm", "true"), SCANNER_PACKAGE_TYPE_PYPI("snyk.scanner.packageType.pypi", "false"), + TEST_CONTINUOUSLY("snyk.scanner.test.continuously","false"), TEST_FREQUENCY_HOURS("snyk.scanner.frequency.hours", "168"), EXTEND_TEST_DEADLINE_HOURS("snyk.scanner.extendTestDeadline.hours", "24"); diff --git a/core/src/main/java/io/snyk/plugins/artifactory/model/TestResult.java b/core/src/main/java/io/snyk/plugins/artifactory/model/TestResult.java index 538f897..e196d3c 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/model/TestResult.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/model/TestResult.java @@ -49,8 +49,8 @@ public ZonedDateTime getTimestamp() { public void write(ArtifactProperties properties) { LOG.info("Writing Snyk properties for package {}", detailsUrl); properties.set(TEST_TIMESTAMP, timestamp.toString()); - properties.set(ISSUE_VULNERABILITIES, getVulnSummary().toString()); - properties.set(ISSUE_LICENSES, getLicenseSummary().toString()); + properties.set(ISSUE_VULNERABILITIES, vulnSummary.toString()); + properties.set(ISSUE_LICENSES, licenseSummary.toString()); properties.set(ISSUE_URL, detailsUrl.toString()); properties.set(ISSUE_URL_PLAINTEXT, " " + detailsUrl); } @@ -63,7 +63,7 @@ public static Optional read(ArtifactProperties properties) { .map(String::trim) .map(URI::create); - if(timestamp.isEmpty() || vulns.isEmpty() || licenses.isEmpty() || detailsUrl.isEmpty()) { + if (timestamp.isEmpty() || vulns.isEmpty() || licenses.isEmpty() || detailsUrl.isEmpty()) { return Optional.empty(); } return Optional.of(new TestResult( diff --git a/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactCache.java b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactCache.java index b4a3d38..e5d3de0 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactCache.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactCache.java @@ -11,7 +11,7 @@ import static org.slf4j.LoggerFactory.getLogger; -public class ArtifactCache { +public class ArtifactCache implements ArtifactResolver { private static final Logger LOG = getLogger(ArtifactCache.class); @@ -24,7 +24,8 @@ public ArtifactCache(Duration testFrequency, Duration extendTestDeadline) { this.extendTestDeadline = extendTestDeadline; } - public MonitoredArtifact getArtifact(ArtifactProperties properties, Supplier fetch) { + @Override + public Optional get(ArtifactProperties properties, Supplier> fetch) { Optional artifact = MonitoredArtifact.read(properties); if (artifact.isEmpty()) { LOG.info("Previous Snyk Test result not available - testing {}", properties.getArtifactPath()); @@ -33,7 +34,7 @@ public MonitoredArtifact getArtifact(ArtifactProperties properties, Supplier fetch) { - return fetch.get().write(properties); + private Optional fetchAndStore(ArtifactProperties properties, Supplier> fetch) { + return fetch.get().map(artifact -> artifact.write(properties)); } private boolean withinTtl(MonitoredArtifact artifact) { @@ -69,5 +70,4 @@ private ZonedDateTime nextTestDue(MonitoredArtifact artifact) { private ZonedDateTime nextTestHardDeadline(MonitoredArtifact artifact) { return nextTestDue(artifact).plus(extendTestDeadline); } - } diff --git a/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactResolver.java b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactResolver.java new file mode 100644 index 0000000..d9e959f --- /dev/null +++ b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ArtifactResolver.java @@ -0,0 +1,12 @@ +package io.snyk.plugins.artifactory.scanner; + +import io.snyk.plugins.artifactory.configuration.properties.ArtifactProperties; +import io.snyk.plugins.artifactory.model.MonitoredArtifact; + +import java.util.Optional; +import java.util.function.Supplier; + +public interface ArtifactResolver { + + Optional get(ArtifactProperties properties, Supplier> fetch); +} diff --git a/core/src/main/java/io/snyk/plugins/artifactory/scanner/ReadOnlyArtifactResolver.java b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ReadOnlyArtifactResolver.java new file mode 100644 index 0000000..02165e9 --- /dev/null +++ b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ReadOnlyArtifactResolver.java @@ -0,0 +1,15 @@ +package io.snyk.plugins.artifactory.scanner; + +import io.snyk.plugins.artifactory.configuration.properties.ArtifactProperties; +import io.snyk.plugins.artifactory.model.MonitoredArtifact; + +import java.util.Optional; +import java.util.function.Supplier; + +public class ReadOnlyArtifactResolver implements ArtifactResolver { + + @Override + public Optional get(ArtifactProperties properties, Supplier> fetch) { + return MonitoredArtifact.read(properties); + } +} diff --git a/core/src/main/java/io/snyk/plugins/artifactory/scanner/ScannerModule.java b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ScannerModule.java index b3714b7..3930dfb 100644 --- a/core/src/main/java/io/snyk/plugins/artifactory/scanner/ScannerModule.java +++ b/core/src/main/java/io/snyk/plugins/artifactory/scanner/ScannerModule.java @@ -5,12 +5,17 @@ import io.snyk.plugins.artifactory.configuration.properties.ArtifactProperties; import io.snyk.plugins.artifactory.configuration.properties.RepositoryArtifactProperties; import io.snyk.plugins.artifactory.exception.CannotScanException; -import io.snyk.plugins.artifactory.model.*; +import io.snyk.plugins.artifactory.model.Ignores; +import io.snyk.plugins.artifactory.model.MonitoredArtifact; +import io.snyk.plugins.artifactory.model.TestResult; +import io.snyk.plugins.artifactory.model.ValidationSettings; import io.snyk.sdk.api.v1.SnykClient; import org.artifactory.fs.FileLayoutInfo; import org.artifactory.repo.RepoPath; import org.artifactory.repo.Repositories; import org.jetbrains.annotations.NotNull; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.annotation.Nonnull; import java.time.Duration; @@ -20,13 +25,13 @@ import static java.util.Objects.requireNonNull; public class ScannerModule { - + private static final Logger LOG = LoggerFactory.getLogger(ScannerModule.class); private final ConfigurationModule configurationModule; private final Repositories repositories; private final MavenScanner mavenScanner; private final NpmScanner npmScanner; private final PythonScanner pythonScanner; - private final ArtifactCache cache; + private final ArtifactResolver artifactResolver; public ScannerModule(@Nonnull ConfigurationModule configurationModule, @Nonnull Repositories repositories, @Nonnull SnykClient snykClient) { this.configurationModule = requireNonNull(configurationModule); @@ -36,23 +41,37 @@ public ScannerModule(@Nonnull ConfigurationModule configurationModule, @Nonnull npmScanner = new NpmScanner(configurationModule, snykClient); pythonScanner = new PythonScanner(configurationModule, snykClient); - cache = new ArtifactCache( + artifactResolver = shouldTestContinuously() ? new ArtifactCache( durationHoursProperty(PluginConfiguration.TEST_FREQUENCY_HOURS, configurationModule), durationHoursProperty(PluginConfiguration.EXTEND_TEST_DEADLINE_HOURS, configurationModule) - ); + ) : new ReadOnlyArtifactResolver(); + } + + public Optional testArtifact(@Nonnull RepoPath repoPath) { + return runTest(repoPath).map(artifact -> artifact.write(properties(repoPath))); } - public void scanArtifact(@Nonnull RepoPath repoPath) { - filter(resolveArtifact(repoPath)); + public void filterAccess(@Nonnull RepoPath repoPath) { + resolveArtifact(repoPath) + .ifPresentOrElse( + this::filter, + () -> LOG.info("No vulnerability info found for {}", repoPath) + ); } - public MonitoredArtifact resolveArtifact(RepoPath repoPath) { - ArtifactProperties properties = new RepositoryArtifactProperties(repoPath, repositories); - return cache.getArtifact(properties, () -> testArtifact(repoPath)); + private Optional resolveArtifact(RepoPath repoPath) { + return artifactResolver.get(properties(repoPath), () -> runTest(repoPath)); } - private @NotNull MonitoredArtifact testArtifact(RepoPath repoPath) { - PackageScanner scanner = getScannerForPackageType(repoPath); + private ArtifactProperties properties(RepoPath repoPath) { + return new RepositoryArtifactProperties(repoPath, repositories); + } + + private @NotNull Optional runTest(RepoPath repoPath) { + return getScannerForPackageType(repoPath).map(scanner -> runTestWith(scanner, repoPath)); + } + + private MonitoredArtifact runTestWith(PackageScanner scanner, RepoPath repoPath) { FileLayoutInfo fileLayoutInfo = repositories.getLayoutInfo(repoPath); TestResult testResult = scanner.scan(fileLayoutInfo, repoPath); return toMonitoredArtifact(testResult, repoPath); @@ -69,14 +88,17 @@ private void filter(MonitoredArtifact artifact) { return new MonitoredArtifact(repoPath.toString(), testResult, ignores); } - protected PackageScanner getScannerForPackageType(RepoPath repoPath) { + protected Optional getScannerForPackageType(RepoPath repoPath) { String path = Optional.ofNullable(repoPath.getPath()) .orElseThrow(() -> new CannotScanException("Path not provided.")); return getScannerForPackageType(path); } - protected PackageScanner getScannerForPackageType(String path) { - Ecosystem ecosystem = Ecosystem.fromPackagePath(path).orElseThrow(() -> new CannotScanException("Artifact is not supported.")); + protected Optional getScannerForPackageType(String path) { + return Ecosystem.fromPackagePath(path).map(this::getScanner); + } + + private PackageScanner getScanner(Ecosystem ecosystem) { if (!configurationModule.getPropertyOrDefault(ecosystem.getConfigProperty()).equals("true")) { throw new CannotScanException(format("Plugin Property \"%s\" is not \"true\".", ecosystem.getConfigProperty().propertyKey())); } @@ -93,6 +115,10 @@ protected PackageScanner getScannerForPackageType(String path) { } } + private boolean shouldTestContinuously() { + return configurationModule.getPropertyOrDefault(PluginConfiguration.TEST_CONTINUOUSLY).equals("true"); + } + private Duration durationHoursProperty(PluginConfiguration property, ConfigurationModule configurationModule) { return Duration.ofHours(Integer.parseInt(configurationModule.getPropertyOrDefault(property))); } diff --git a/core/src/test/java/io/snyk/plugins/artifactory/model/MonitoredArtifactTest.java b/core/src/test/java/io/snyk/plugins/artifactory/model/MonitoredArtifactTest.java index efb9556..a18572d 100644 --- a/core/src/test/java/io/snyk/plugins/artifactory/model/MonitoredArtifactTest.java +++ b/core/src/test/java/io/snyk/plugins/artifactory/model/MonitoredArtifactTest.java @@ -5,12 +5,10 @@ import org.junit.jupiter.api.Test; import java.net.URI; -import java.util.Optional; import java.util.stream.Stream; import static io.snyk.plugins.artifactory.configuration.properties.ArtifactProperty.*; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.assertj.core.api.Assertions.assertThat; class MonitoredArtifactTest { @@ -28,13 +26,13 @@ void write_firstTime() { artifact.write(properties); - assertEquals("1 critical, 0 high, 0 medium, 0 low", properties.get(ISSUE_VULNERABILITIES).get()); - assertEquals("0 critical, 0 high, 1 medium, 0 low", properties.get(ISSUE_LICENSES).get()); - assertEquals("https://app.snyk.io/package/electron/1.0.0", properties.get(ISSUE_URL).get()); - assertEquals("false", properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD).get()); - assertEquals("", properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD_INFO).get()); - assertEquals("false", properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD).get()); - assertEquals("", properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD_INFO).get()); + assertThat(properties.get(ISSUE_VULNERABILITIES)).contains("1 critical, 0 high, 0 medium, 0 low"); + assertThat(properties.get(ISSUE_LICENSES)).contains("0 critical, 0 high, 1 medium, 0 low"); + assertThat(properties.get(ISSUE_URL)).contains("https://app.snyk.io/package/electron/1.0.0"); + assertThat(properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD)).contains("false"); + assertThat(properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD_INFO)).contains(""); + assertThat(properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD)).contains("false"); + assertThat(properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD_INFO)).contains(""); } @Test @@ -51,7 +49,7 @@ void write_createsPlainTextLinkAsAWorkaroundForArtifactoryLinkRenderGlitch() { artifact.write(properties); - assertEquals(" https://app.snyk.io/package/electron/1.0.0", properties.get(ISSUE_URL_PLAINTEXT).get()); + assertThat(properties.get(ISSUE_URL_PLAINTEXT)).contains(" https://app.snyk.io/package/electron/1.0.0"); } @Test @@ -73,13 +71,13 @@ void write_whenPropertiesHadBeenSetBefore() { artifact.write(properties); - assertEquals("0 critical, 1 high, 0 medium, 0 low", properties.get(ISSUE_VULNERABILITIES).get()); - assertEquals("0 critical, 0 high, 0 medium, 1 low", properties.get(ISSUE_LICENSES).get()); - assertEquals("https://app.snyk.io/package/electron/2.0.0", properties.get(ISSUE_URL).get()); - assertEquals("true", properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD).get()); - assertEquals("issue ignored by prodsec", properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD_INFO).get()); - assertEquals("true", properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD).get()); - assertEquals("issue ignored by legal", properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD_INFO).get()); + assertThat(properties.get(ISSUE_VULNERABILITIES)).contains("0 critical, 1 high, 0 medium, 0 low"); + assertThat(properties.get(ISSUE_LICENSES)).contains("0 critical, 0 high, 0 medium, 1 low"); + assertThat(properties.get(ISSUE_URL)).contains("https://app.snyk.io/package/electron/2.0.0"); + assertThat(properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD)).contains("true"); + assertThat(properties.get(ISSUE_VULNERABILITIES_FORCE_DOWNLOAD_INFO)).contains("issue ignored by prodsec"); + assertThat(properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD)).contains("true"); + assertThat(properties.get(ISSUE_LICENSES_FORCE_DOWNLOAD_INFO)).contains("issue ignored by legal"); } @Test @@ -95,19 +93,31 @@ void read_whenPersistedBefore() { ); originalArtifact.write(properties); - Optional retrievedArtifact = MonitoredArtifact.read(properties); - assertTrue(retrievedArtifact.isPresent()); - assertEquals(originalArtifact, retrievedArtifact.get()); + assertThat(MonitoredArtifact.read(properties)).contains(originalArtifact); } @Test void read_whenPropertiesMissing() { FakeArtifactProperties properties = new FakeArtifactProperties("electron"); - Optional retrievedArtifact = MonitoredArtifact.read(properties); + assertThat(MonitoredArtifact.read(properties)).isEmpty(); + } + + @Test + void read_whenTimestampMissing() { + FakeArtifactProperties properties = new FakeArtifactProperties("electron"); + new MonitoredArtifact("electron", + new TestResult( + IssueSummary.from(Stream.of(Severity.CRITICAL)), + IssueSummary.from(Stream.of(Severity.MEDIUM)), + URI.create("https://app.snyk.io/package/electron/1.0.0") + ), + new Ignores() + ).write(properties); + properties.set(TEST_TIMESTAMP, ""); - assertTrue(retrievedArtifact.isEmpty()); + assertThat(MonitoredArtifact.read(properties)).isEmpty(); } @Test @@ -121,9 +131,9 @@ void read_whenPropertiesMalformed() { ), new Ignores() ).write(properties); - properties.set(TEST_TIMESTAMP, "not a valid timestamp"); + properties.set(ISSUE_VULNERABILITIES, "not a valid format"); - assertTrue(MonitoredArtifact.read(properties).isEmpty()); + assertThat(MonitoredArtifact.read(properties)).isEmpty(); } @Test @@ -139,6 +149,6 @@ void read_whenUrlHasWhitespaces() { ).write(properties); properties.set(ISSUE_URL, " https://app.snyk.io/package/electron/1.0.0 "); - assertEquals(Optional.of(artifact), MonitoredArtifact.read(properties)); + assertThat(MonitoredArtifact.read(properties)).contains(artifact); } } diff --git a/core/src/test/java/io/snyk/plugins/artifactory/scanner/ArtifactCacheTest.java b/core/src/test/java/io/snyk/plugins/artifactory/scanner/ArtifactCacheTest.java index d709909..4477c8c 100644 --- a/core/src/test/java/io/snyk/plugins/artifactory/scanner/ArtifactCacheTest.java +++ b/core/src/test/java/io/snyk/plugins/artifactory/scanner/ArtifactCacheTest.java @@ -32,36 +32,36 @@ void setUp() { properties = new FakeArtifactProperties(name); } - MonitoredArtifact fetch() { + Optional fetch() { return anArtifact(ZonedDateTime.now()); } - MonitoredArtifact failToFetch() { + Optional failToFetch() { throw new RuntimeException("Failed to fetch artifact"); } @Test void getArtifact_whenNothingCached() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(1), Duration.ofDays(1)); - assertNotNull(cache.getArtifact(properties, this::fetch)); + assertTrue(cache.get(properties, this::fetch).isPresent()); } @Test void getArtifact_whenFreshlyCached() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(1), Duration.ofDays(1)); - MonitoredArtifact artifact = cache.getArtifact(properties, this::fetch); + Optional artifact = cache.get(properties, this::fetch); - assertEquals(artifact, cache.getArtifact(properties, this::fetch)); + assertEquals(artifact, cache.get(properties, this::fetch)); } @Test void getArtifact_whenCacheStale() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(1), Duration.ofDays(1)); - MonitoredArtifact oldArtifact = cache.getArtifact(properties, () -> anArtifact(ZonedDateTime.now().minusDays(2))); + Optional oldArtifact = cache.get(properties, () -> anArtifact(ZonedDateTime.now().minusDays(2))); - MonitoredArtifact newArtifact = cache.getArtifact(properties, this::fetch); + Optional newArtifact = cache.get(properties, this::fetch); assertNotEquals(newArtifact, oldArtifact); } @@ -69,9 +69,9 @@ void getArtifact_whenCacheStale() { @Test void getArtifact_whenTestFrequencyIs0_alwaysFetches() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(0), Duration.ofDays(1)); - MonitoredArtifact oldArtifact = cache.getArtifact(properties, () -> anArtifact(ZonedDateTime.now().plusDays(1))); + Optional oldArtifact = cache.get(properties, () -> anArtifact(ZonedDateTime.now().plusDays(1))); - MonitoredArtifact newArtifact = cache.getArtifact(properties, this::fetch); + Optional newArtifact = cache.get(properties, this::fetch); assertNotEquals(newArtifact, oldArtifact); } @@ -79,9 +79,9 @@ void getArtifact_whenTestFrequencyIs0_alwaysFetches() { @Test void getArtifact_whenFetchFails_reliesOnStaleCache() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(1), Duration.ofDays(1)); - MonitoredArtifact staleArtifact = cache.getArtifact(properties, () -> anArtifact(ZonedDateTime.now().minusHours(2))); + Optional staleArtifact = cache.get(properties, () -> anArtifact(ZonedDateTime.now().minusHours(2))); - MonitoredArtifact cachedArtifact = cache.getArtifact(properties, this::failToFetch); + Optional cachedArtifact = cache.get(properties, this::failToFetch); assertEquals(staleArtifact, cachedArtifact); } @@ -89,20 +89,20 @@ void getArtifact_whenFetchFails_reliesOnStaleCache() { @Test void getArtifact_whenFetchFailsForTooLong_throws() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(1), Duration.ofDays(1)); - cache.getArtifact(properties, () -> anArtifact(ZonedDateTime.now().minusDays(2))); + cache.get(properties, () -> anArtifact(ZonedDateTime.now().minusDays(2))); - assertThrows(RuntimeException.class, () -> cache.getArtifact(properties, this::failToFetch), "Failed to fetch artifact"); + assertThrows(RuntimeException.class, () -> cache.get(properties, this::failToFetch), "Failed to fetch artifact"); } @Test void getArtifact_whenFetchFailsAndNoCache_throws() { ArtifactCache cache = new ArtifactCache(Duration.ofHours(1), Duration.ofDays(1)); - assertThrows(RuntimeException.class, () -> cache.getArtifact(properties, this::failToFetch), "Failed to fetch artifact"); + assertThrows(RuntimeException.class, () -> cache.get(properties, this::failToFetch), "Failed to fetch artifact"); } - private MonitoredArtifact anArtifact(ZonedDateTime timestamp) { + private Optional anArtifact(ZonedDateTime timestamp) { TestResult recentTestResult = new TestResult(timestamp, IssueSummary.from(Stream.empty()), IssueSummary.from(Stream.empty()), URI.create("https://snyk.io")); - return new MonitoredArtifact(name, recentTestResult, new Ignores()); + return Optional.of(new MonitoredArtifact(name, recentTestResult, new Ignores())); } } diff --git a/core/src/test/java/io/snyk/plugins/artifactory/scanner/ScannerModuleTest.java b/core/src/test/java/io/snyk/plugins/artifactory/scanner/ScannerModuleTest.java index 25e30bd..a13569f 100644 --- a/core/src/test/java/io/snyk/plugins/artifactory/scanner/ScannerModuleTest.java +++ b/core/src/test/java/io/snyk/plugins/artifactory/scanner/ScannerModuleTest.java @@ -108,7 +108,7 @@ void shouldUseConfiguredTimeoutForAPIRequests() throws Exception { // Using try-catch as assertThrows does not let us check the cause. try { - spyScanner.scanArtifact(repoPath); + spyScanner.testArtifact(repoPath); fail("Expected SnykAPIFailureException for timeout but no exception was thrown."); } catch (SnykAPIFailureException e) { Throwable cause = e.getCause(); @@ -129,7 +129,7 @@ void testScanNpmItem_noVulns() throws Exception { when(repoPath.getPath()).thenReturn("myArtifact.tgz"); when(repoPath.toString()).thenReturn("npm:minimist/-/minimist-1.2.6.tgz"); - MonitoredArtifact result = spyScanner.resolveArtifact(repoPath); + MonitoredArtifact result = spyScanner.testArtifact(repoPath).get(); assertEquals(0, result.getTestResult().getVulnSummary().getTotalCount()); } @@ -146,7 +146,7 @@ void testScanNpmItem_withVulns() throws Exception { when(repoPath.getPath()).thenReturn("myArtifact.tgz"); when(repoPath.toString()).thenReturn("npm:lodash/-/lodash-4.17.15.tgz"); - MonitoredArtifact result = spyScanner.resolveArtifact(repoPath); + MonitoredArtifact result = spyScanner.testArtifact(repoPath).get(); assertTrue(result.getTestResult().getVulnSummary().getTotalCount() > 0); } @@ -162,7 +162,7 @@ void testScanMavenItem_noVulns() throws Exception { RepoPath repoPath = testSetup.repoPath; when(repoPath.getPath()).thenReturn("myArtifact.jar"); - MonitoredArtifact result = spyScanner.resolveArtifact(repoPath); + MonitoredArtifact result = spyScanner.testArtifact(repoPath).get(); assertEquals(0, result.getTestResult().getVulnSummary().getTotalCount()); } @@ -178,7 +178,7 @@ void testScanMavenItem_withVulns() throws Exception { RepoPath repoPath = testSetup.repoPath; when(repoPath.getPath()).thenReturn("myArtifact.jar"); - MonitoredArtifact result = spyScanner.resolveArtifact(repoPath); + MonitoredArtifact result = spyScanner.testArtifact(repoPath).get(); assertTrue(result.getTestResult().getVulnSummary().getTotalCount() > 0); } @@ -193,7 +193,7 @@ void testScanPythonItem_noVulns() throws Exception { RepoPath repoPath = testSetup.repoPath; when(repoPath.getPath()).thenReturn("myArtifact.whl"); - MonitoredArtifact result = spyScanner.resolveArtifact(repoPath); + MonitoredArtifact result = spyScanner.testArtifact(repoPath).get(); assertEquals(0, result.getTestResult().getVulnSummary().getTotalCount()); } @@ -208,7 +208,7 @@ void testScanPythonItem_withVulns() throws Exception { RepoPath repoPath = testSetup.repoPath; when(repoPath.getPath()).thenReturn("myArtifact.whl"); - MonitoredArtifact result = spyScanner.resolveArtifact(repoPath); + MonitoredArtifact result = spyScanner.testArtifact(repoPath).get(); assertEquals(6, result.getTestResult().getVulnSummary().getTotalCount()); } }