diff --git a/src/DockerUpdateGuard/Images/VulnerabilityEnrichmentService.cs b/src/DockerUpdateGuard/Images/VulnerabilityEnrichmentService.cs index 993dd96..0fb2941 100644 --- a/src/DockerUpdateGuard/Images/VulnerabilityEnrichmentService.cs +++ b/src/DockerUpdateGuard/Images/VulnerabilityEnrichmentService.cs @@ -147,6 +147,33 @@ private static List DeduplicateAdvisories(IReadOnlyLi .ToList(); } + /// + /// Select the finding that represents a finding identity, preferring the active row and otherwise the most recently resolved one + /// + /// Findings sharing the same identity key + /// Representative finding + private static VulnerabilityFinding SelectRepresentativeFinding(IEnumerable findings) + { + return findings.OrderByDescending(entity => entity.IsActive) + .ThenByDescending(entity => entity.ResolvedAtUtc) + .ThenBy(entity => entity.DetectedAtUtc) + .First(); + } + + /// + /// Reactivate a resolved finding because its advisory was reported again + /// + /// Finding to reactivate + /// Scan run that reported the advisory again + private static void ReactivateFinding(VulnerabilityFinding finding, ScanRun scanRun) + { + finding.IsActive = true; + finding.ResolvedAtUtc = null; + finding.ResolvedByScanRunId = null; + finding.ScanRunId = scanRun.Id; + finding.DetectedAtUtc = DateTimeOffset.UtcNow; + } + /// /// Copy the mutable advisory values onto a finding /// @@ -182,22 +209,30 @@ private async Task UpsertVulnerabilityFindingsAsync(ScanRun scanRun, IReadOnlyList advisories, CancellationToken cancellationToken) { - var activeFindings = await _dbContext.VulnerabilityFindings.Where(entity => entity.ImageVersionId == image.Id && entity.IsActive) - .ToListAsync(cancellationToken) - .ConfigureAwait(false); - - var activeFindingLookup = activeFindings.GroupBy(entity => CreateFindingKey(entity.AdvisoryId, entity.AffectedPackage)) - .ToDictionary(findingGroup => findingGroup.Key, - findingGroup => findingGroup.OrderBy(entity => entity.DetectedAtUtc) - .First()); + var existingFindings = await _dbContext.VulnerabilityFindings.Where(entity => entity.ImageVersionId == image.Id) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + + // Rows that were active before this run are the only candidates for the stale pass below + var previouslyActiveFindings = existingFindings.Where(entity => entity.IsActive) + .ToList(); + + var findingLookup = existingFindings.GroupBy(entity => CreateFindingKey(entity.AdvisoryId, entity.AffectedPackage)) + .ToDictionary(findingGroup => findingGroup.Key, + SelectRepresentativeFinding); var source = MapSource(_optionsMonitor.CurrentValue.Vulnerabilities.Provider); var dedupedAdvisories = DeduplicateAdvisories(advisories); var matchedFindingIds = new HashSet(); foreach (var advisory in dedupedAdvisories) { - if (activeFindingLookup.TryGetValue(CreateFindingKey(advisory.AdvisoryId, advisory.AffectedPackage), out var existingFinding)) + if (findingLookup.TryGetValue(CreateFindingKey(advisory.AdvisoryId, advisory.AffectedPackage), out var existingFinding)) { + if (existingFinding.IsActive == false) + { + ReactivateFinding(existingFinding, scanRun); + } + UpdateFindingFromAdvisory(existingFinding, advisory, source); matchedFindingIds.Add(existingFinding.Id); @@ -216,7 +251,7 @@ private async Task UpsertVulnerabilityFindingsAsync(ScanRun scanRun, _dbContext.VulnerabilityFindings.Add(newFinding); } - foreach (var staleFinding in activeFindings.Where(entity => matchedFindingIds.Contains(entity.Id) == false)) + foreach (var staleFinding in previouslyActiveFindings.Where(entity => matchedFindingIds.Contains(entity.Id) == false)) { staleFinding.IsActive = false; staleFinding.ResolvedAtUtc = DateTimeOffset.UtcNow; diff --git a/src/Tests/DockerUpdateGuard.Tests/VulnerabilityEnrichmentServiceTests.cs b/src/Tests/DockerUpdateGuard.Tests/VulnerabilityEnrichmentServiceTests.cs index aad5d96..c7202bd 100644 --- a/src/Tests/DockerUpdateGuard.Tests/VulnerabilityEnrichmentServiceTests.cs +++ b/src/Tests/DockerUpdateGuard.Tests/VulnerabilityEnrichmentServiceTests.cs @@ -783,5 +783,335 @@ await service.RefreshAsync(ScanTriggerSource.Manual, CancellationToken.None) } } + /// + /// Verify a resolved finding whose advisory is reported again is reactivated instead of stored as a second row + /// + /// Task + [TestMethod] + public async Task VulnerabilityEnrichmentServiceRefreshAsyncReportedResolvedFindingIsReactivatedInsteadOfDuplicatedAsync() + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + var imageCatalogRepository = new ImageCatalogRepository(dbContext); + var currentImageVersion = await imageCatalogRepository.GetOrCreateImageVersionAsync("docker.io", + "company/app", + "1.0.0", + "sha256:app", + cancellationToken: CancellationToken.None) + .ConfigureAwait(false); + var previousScanRun = new ScanRun + { + Type = ScanRunType.Vulnerability, + Status = ScanRunStatus.Succeeded, + TriggerSource = ScanTriggerSource.Scheduled, + StartedAtUtc = DateTimeOffset.UtcNow.AddDays(-2), + CompletedAtUtc = DateTimeOffset.UtcNow.AddDays(-2), + }; + var detectedAtUtc = DateTimeOffset.UtcNow.AddDays(-3); + var resolvedFinding = new VulnerabilityFinding + { + ImageVersionId = currentImageVersion.Id, + AdvisoryId = "CVE-2026-3000", + Title = "Reappearing advisory", + Severity = VulnerabilitySeverity.Low, + Source = VulnerabilitySource.Trivy, + AffectedPackage = "openssl", + IsActive = false, + DetectedAtUtc = detectedAtUtc, + ResolvedAtUtc = DateTimeOffset.UtcNow.AddDays(-2), + ResolvedByScanRunId = previousScanRun.Id, + }; + var vulnerabilityProvider = Substitute.For(); + var vulnerabilityProviderResolver = Substitute.For(); + + vulnerabilityProviderResolver.Resolve().Returns(vulnerabilityProvider); + + var optionsMonitor = new TestOptionsMonitor(new DockerUpdateGuardOptions + { + Vulnerabilities = new VulnerabilityOptions + { + Enabled = true, + Provider = VulnerabilityProviderKind.Trivy, + }, + }); + + dbContext.ObservedImages.Add(new ObservedImage + { + Name = "Company App", + CurrentImageVersionId = currentImageVersion.Id, + }); + dbContext.ScanRuns.Add(previousScanRun); + dbContext.VulnerabilityFindings.Add(resolvedFinding); + await dbContext.SaveChangesAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + + vulnerabilityProvider.GetVulnerabilitiesAsync(Arg.Any(), Arg.Any()) + .Returns(ExternalOperationResult>.Succeeded([ + new VulnerabilityAdvisoryData + { + AdvisoryId = "CVE-2026-3000", + Title = "Reappearing advisory", + Severity = VulnerabilitySeverity.High, + AffectedPackage = "openssl", + }, + ])); + + var service = new VulnerabilityEnrichmentService(new ApplicationTelemetry(), + dbContext, + new ImageReferenceParser(), + new LiveImageInventoryQueryService(dbContext), + new TestLogger(), + optionsMonitor, + vulnerabilityProviderResolver); + + await service.RefreshAsync(ScanTriggerSource.Manual, CancellationToken.None) + .ConfigureAwait(false); + + var findings = await dbContext.VulnerabilityFindings.ToListAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + var reactivatedFinding = findings.Single(); + var currentScanRun = await dbContext.ScanRuns.SingleAsync(entity => entity.Id != previousScanRun.Id, TestContext.CancellationToken) + .ConfigureAwait(false); + + Assert.HasCount(1, + findings, + "A resolved finding that is reported again must not create a second row for the same advisory and package"); + Assert.AreEqual(resolvedFinding.Id, + reactivatedFinding.Id, + "A resolved finding that is reported again must be reactivated in place"); + Assert.IsTrue(reactivatedFinding.IsActive, "A resolved finding that is reported again must become active"); + Assert.IsNull(reactivatedFinding.ResolvedAtUtc, "Reactivation must clear the resolution timestamp"); + Assert.IsNull(reactivatedFinding.ResolvedByScanRunId, "Reactivation must clear the resolving scan run"); + Assert.AreEqual(currentScanRun.Id, + reactivatedFinding.ScanRunId, + "Reactivation must stamp the scan run that reported the advisory again"); + Assert.IsGreaterThan(detectedAtUtc, + reactivatedFinding.DetectedAtUtc, + "Reactivation must start a new active period with a fresh detection timestamp"); + Assert.AreEqual(VulnerabilitySeverity.High, + reactivatedFinding.Severity, + "Reactivated findings must take over the latest reported severity"); + } + } + } + + /// + /// Verify a finding reactivated during a run is not deactivated again by the stale pass of the same run + /// + /// Task + [TestMethod] + public async Task VulnerabilityEnrichmentServiceRefreshAsyncReactivatedFindingSurvivesStalePassOfSameRunAsync() + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + var imageCatalogRepository = new ImageCatalogRepository(dbContext); + var currentImageVersion = await imageCatalogRepository.GetOrCreateImageVersionAsync("docker.io", + "company/app", + "1.0.0", + "sha256:app", + cancellationToken: CancellationToken.None) + .ConfigureAwait(false); + var resolvedFinding = new VulnerabilityFinding + { + ImageVersionId = currentImageVersion.Id, + AdvisoryId = "CVE-2026-4000", + Title = "Reappearing advisory", + Severity = VulnerabilitySeverity.Medium, + Source = VulnerabilitySource.Trivy, + AffectedPackage = "openssl", + IsActive = false, + DetectedAtUtc = DateTimeOffset.UtcNow.AddDays(-3), + ResolvedAtUtc = DateTimeOffset.UtcNow.AddDays(-2), + }; + var disappearingFinding = new VulnerabilityFinding + { + ImageVersionId = currentImageVersion.Id, + AdvisoryId = "CVE-2026-5000", + Title = "Disappearing advisory", + Severity = VulnerabilitySeverity.Low, + Source = VulnerabilitySource.Trivy, + AffectedPackage = "zlib", + IsActive = true, + DetectedAtUtc = DateTimeOffset.UtcNow.AddDays(-1), + }; + var vulnerabilityProvider = Substitute.For(); + var vulnerabilityProviderResolver = Substitute.For(); + + vulnerabilityProviderResolver.Resolve().Returns(vulnerabilityProvider); + + var optionsMonitor = new TestOptionsMonitor(new DockerUpdateGuardOptions + { + Vulnerabilities = new VulnerabilityOptions + { + Enabled = true, + Provider = VulnerabilityProviderKind.Trivy, + }, + }); + + dbContext.ObservedImages.Add(new ObservedImage + { + Name = "Company App", + CurrentImageVersionId = currentImageVersion.Id, + }); + dbContext.VulnerabilityFindings.Add(resolvedFinding); + dbContext.VulnerabilityFindings.Add(disappearingFinding); + await dbContext.SaveChangesAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + + vulnerabilityProvider.GetVulnerabilitiesAsync(Arg.Any(), Arg.Any()) + .Returns(ExternalOperationResult>.Succeeded([ + new VulnerabilityAdvisoryData + { + AdvisoryId = "CVE-2026-4000", + Title = "Reappearing advisory", + Severity = VulnerabilitySeverity.Medium, + AffectedPackage = "openssl", + }, + ])); + + var service = new VulnerabilityEnrichmentService(new ApplicationTelemetry(), + dbContext, + new ImageReferenceParser(), + new LiveImageInventoryQueryService(dbContext), + new TestLogger(), + optionsMonitor, + vulnerabilityProviderResolver); + + await service.RefreshAsync(ScanTriggerSource.Manual, CancellationToken.None) + .ConfigureAwait(false); + + var findings = await dbContext.VulnerabilityFindings.ToListAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + var persistedReactivatedFinding = findings.Single(entity => entity.Id == resolvedFinding.Id); + var persistedDisappearingFinding = findings.Single(entity => entity.Id == disappearingFinding.Id); + + Assert.HasCount(2, + findings, + "Reactivating a resolved finding must not add rows to the findings of the image version"); + Assert.IsTrue(persistedReactivatedFinding.IsActive, "A finding reactivated during a run must not be deactivated by the stale pass of the same run"); + Assert.IsNull(persistedReactivatedFinding.ResolvedAtUtc, "A finding reactivated during a run must keep its cleared resolution timestamp"); + Assert.IsFalse(persistedDisappearingFinding.IsActive, "Findings that were active before the run and are no longer reported must be resolved"); + Assert.IsNotNull(persistedDisappearingFinding.ResolvedAtUtc, "Findings resolved by the stale pass must record a resolution timestamp"); + } + } + } + + /// + /// Verify a legacy state with an active and a resolved row for one advisory keeps updating the active row + /// + /// Task + [TestMethod] + public async Task VulnerabilityEnrichmentServiceRefreshAsyncDuplicatedLegacyFindingUpdatesActiveRowAsync() + { + using (var database = new SqliteTestDatabase()) + { + var dbContext = database.CreateDbContext(); + + await using (dbContext.ConfigureAwait(false)) + { + var imageCatalogRepository = new ImageCatalogRepository(dbContext); + var currentImageVersion = await imageCatalogRepository.GetOrCreateImageVersionAsync("docker.io", + "company/app", + "1.0.0", + "sha256:app", + cancellationToken: CancellationToken.None) + .ConfigureAwait(false); + var legacyResolvedFinding = new VulnerabilityFinding + { + ImageVersionId = currentImageVersion.Id, + AdvisoryId = "CVE-2026-6000", + Title = "Duplicated advisory", + Severity = VulnerabilitySeverity.Low, + Source = VulnerabilitySource.Trivy, + AffectedPackage = "openssl", + IsActive = false, + DetectedAtUtc = DateTimeOffset.UtcNow.AddDays(-5), + ResolvedAtUtc = DateTimeOffset.UtcNow.AddDays(-4), + }; + var activeFinding = new VulnerabilityFinding + { + ImageVersionId = currentImageVersion.Id, + AdvisoryId = "CVE-2026-6000", + Title = "Duplicated advisory", + Severity = VulnerabilitySeverity.Low, + Source = VulnerabilitySource.Trivy, + AffectedPackage = "openssl", + IsActive = true, + DetectedAtUtc = DateTimeOffset.UtcNow.AddDays(-3), + }; + var vulnerabilityProvider = Substitute.For(); + var vulnerabilityProviderResolver = Substitute.For(); + + vulnerabilityProviderResolver.Resolve().Returns(vulnerabilityProvider); + + var optionsMonitor = new TestOptionsMonitor(new DockerUpdateGuardOptions + { + Vulnerabilities = new VulnerabilityOptions + { + Enabled = true, + Provider = VulnerabilityProviderKind.Trivy, + }, + }); + + dbContext.ObservedImages.Add(new ObservedImage + { + Name = "Company App", + CurrentImageVersionId = currentImageVersion.Id, + }); + dbContext.VulnerabilityFindings.Add(legacyResolvedFinding); + dbContext.VulnerabilityFindings.Add(activeFinding); + await dbContext.SaveChangesAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + + vulnerabilityProvider.GetVulnerabilitiesAsync(Arg.Any(), Arg.Any()) + .Returns(ExternalOperationResult>.Succeeded([ + new VulnerabilityAdvisoryData + { + AdvisoryId = "CVE-2026-6000", + Title = "Duplicated advisory", + Severity = VulnerabilitySeverity.Critical, + AffectedPackage = "openssl", + }, + ])); + + var service = new VulnerabilityEnrichmentService(new ApplicationTelemetry(), + dbContext, + new ImageReferenceParser(), + new LiveImageInventoryQueryService(dbContext), + new TestLogger(), + optionsMonitor, + vulnerabilityProviderResolver); + + await service.RefreshAsync(ScanTriggerSource.Manual, CancellationToken.None) + .ConfigureAwait(false); + + var findings = await dbContext.VulnerabilityFindings.ToListAsync(TestContext.CancellationToken) + .ConfigureAwait(false); + var persistedActiveFinding = findings.Single(entity => entity.Id == activeFinding.Id); + var persistedLegacyFinding = findings.Single(entity => entity.Id == legacyResolvedFinding.Id); + + Assert.HasCount(2, + findings, + "Legacy duplicates must not grow the findings table when the advisory is reported again"); + Assert.IsTrue(persistedActiveFinding.IsActive, "The active row must be preferred over a resolved row of the same finding identity"); + Assert.AreEqual(VulnerabilitySeverity.Critical, + persistedActiveFinding.Severity, + "The active row must take over the latest reported severity"); + Assert.IsFalse(persistedLegacyFinding.IsActive, "A legacy resolved duplicate must stay resolved when an active row exists for the same finding identity"); + Assert.AreEqual(VulnerabilitySeverity.Low, + persistedLegacyFinding.Severity, + "A legacy resolved duplicate must not be updated from the reported advisory"); + } + } + } + #endregion // Methods } \ No newline at end of file