From 867076151d325b1ce94c2b9bfdba1c55cc7ccaa2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 19:30:09 +0000 Subject: [PATCH] Reactivate resolved vulnerability findings instead of duplicating rows The upsert built its match lookup from active findings only, so an advisory that was resolved in an earlier run and reappeared was inserted as a new row while the old row stayed behind as a tombstone. The lookup now covers all findings of the image version and prefers the active row, falling back to the most recently resolved one. A matched resolved row is reactivated in place with a cleared resolution state and the current scan run, and the stale pass only considers rows that were already active when the run started. Closes #204. --- .../Images/VulnerabilityEnrichmentService.cs | 55 ++- .../VulnerabilityEnrichmentServiceTests.cs | 330 ++++++++++++++++++ 2 files changed, 375 insertions(+), 10 deletions(-) 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