[BUG] Runtime container scans and vulnerability refreshes can overlap; add the per-subject scan guard #292

Summary

Observed-image scans are serialized per observed image within the process, but runtime container scans and vulnerability refreshes have no such guard. A manual run from the UI and the scheduled background run of the same kind can therefore work on the same Docker instance or the same image version at the same time, each reading the same prior state and writing snapshots and findings independently. The product decision is to give both the same per-subject overlap guard that observed-image scans already have.

Area

• docs/areas/scan-engine.md (added in [Docs] Describe the current behavior by area #263) — Overlapping scans, Open question 3
• docs/areas/vulnerabilities.md, docs/areas/docker-instances.md (behavior of the affected scans; informational)
• Security area: None

Current behavior

Existing guard (observed images) — src/DockerUpdateGuard/Images/ImageScanOrchestrator.cs:

• :38-43 a static Lock plus a Dictionary<Guid, ObservedImageScanLockEntry> of reference-counted SemaphoreSlims (Images/Data/ObservedImageScanLockEntry.cs);
• :167-207 AcquireObservedImageScanLock / ReleaseObservedImageScanLockReference;
• :677-796 ScanAsync waits on the image's semaphore, runs the full scan, releases it and drops the entry when no one else holds or waits for it. A second request waits and then runs in full (not skipped, not merged).

No guard for runtime container scans — Images/RuntimeContainerScanOrchestrator.cs:

• ScanAllAsync (:1174-1233) iterates the enabled instances and calls ScanInstanceAsync (:438) for each, which creates a RuntimeContainer scan run, discovers containers, writes snapshots and deactivates/creates update findings (DeactivateSupersededRuntimeFindingsAsync :802, DeactivateRemovedRuntimeFindingsAsync :825, CreateRuntimeFindingAsync :850).
• Started by the scheduled job (Images/RuntimeContainerRefreshBackgroundService.cs) and by Check image updates in the UI (Components/Pages/RuntimeContainers.razor.cs:378, ScanAllAsync(ScanTriggerSource.Manual)), each in its own scope/DbContext.

No guard for vulnerability refreshes — Images/VulnerabilityEnrichmentService.cs:

• RefreshAsync (:537-554, all live images) and RefreshImageVersionAsync (:557-579, one image) both go through RunRefreshAsync (:397-535), which per image calls ApplyImageVulnerabilitiesAsync → UpsertVulnerabilityFindingsAsync (:219): it loads the existing findings of the image version, matches advisories, inserts new findings and deactivates missing ones, then saves.
• Started by the scheduled job (Images/VulnerabilityRefreshBackgroundService.cs), Refresh vulnerabilities (Components/Pages/ScanHistory.razor.cs:194) and the per-image Rescan (Components/Shared/VulnerabilityRescanControl.razor.cs:78).

Failure scenario: the scheduled vulnerability refresh is processing image version X while a user clicks Rescan for X. Both runs load X's findings before either saves, both find advisory CVE-…/package P missing and both insert a finding → two active findings for the same advisory and package (there is no unique index, see #14), counted twice in the gauge and the Vulnerabilities page. Analogously, a manual Check image updates during the scheduled runtime refresh of the same instance lets both runs deactivate and recreate the same containers' update findings from the same prior state, producing duplicate active findings or deactivating the other run's fresh ones.

Expected behavior

Decision (product manager): runtime container scans and vulnerability refreshes get the same per-subject overlap guard as observed-image scans.

• Runtime container scans: scans of the same Docker instance never run at the same time within one application process. The subject is the Docker instance (its database ID). A second scan of an instance that is being scanned waits until the first has finished and then runs in full; it is neither skipped nor merged. Scans of different instances may run at the same time.
• Vulnerability refreshes: the vulnerability work on the same image version never runs at the same time within one process. The subject is the image version: the per-image step (read existing findings, apply provider results, save) of one run waits while another run is processing the same image version, then runs in full. Different image versions may be processed at the same time; provider-call parallelism inside one refresh (MaxParallelScans) is unchanged.
• The fleet-wide stale-finding deactivation of a full refresh (DeactivateStaleFindingsAsync) does not run concurrently with another full refresh's deactivation.
• Waiting honors cancellation (host shutdown or a cancelled UI action ends the wait without starting the work).
• Scan runs are still recorded as today, one per run: a runtime container scan run is created after the instance lock has been acquired (so its start time is when its work starts); a vulnerability scan run is created at the start of the refresh, before any per-image wait, and a wait counts towards its duration.
• The guard stays process-local (single-instance assumption unchanged).

Acceptance criteria

• Two concurrent ScanInstanceAsync calls for the same Docker instance execute strictly one after the other; for different instances they may overlap. Tests in RuntimeContainerScanOrchestratorTests.cs, e.g. RuntimeContainerScanOrchestratorConcurrentScansOfSameInstanceRunSequentially, RuntimeContainerScanOrchestratorConcurrentScansOfDifferentInstancesMayOverlap (use a blocking fake IDockerInstanceClient to hold the first scan, like the concurrency helpers in src/Tests/DockerUpdateGuard.Tests/Helper/).
• Two concurrent refreshes touching the same image version apply their results sequentially and leave exactly one active finding per advisory and package: VulnerabilityEnrichmentServiceConcurrentRefreshesOfSameImageVersionDoNotDuplicateFindings (e.g. with Helper/ConcurrencyTrackingVulnerabilityProvider.cs).
• A waiting scan is cancelled by its token without doing work: …WaitingScanCancelledDoesNotRun.
• The lock registry does not grow without bound (entries are removed when no one holds or waits), covered by a test like the existing observed-image one.
• Coverage gate passes; assertions have messages; no real-clock waits (use TaskCompletionSource-based fakes).
• docs/ARCHITECTURE.md (background job engine / overlap rules) updated.
• The area document docs/areas/scan-engine.md (Overlapping scans) describes the new behavior and its Known gaps entry for this issue is removed.

Implementation notes

• Recommended: extract the reference-counted keyed semaphore from ImageScanOrchestrator (:38-43, :167-207, Images/Data/ObservedImageScanLockEntry.cs) into one reusable internal class (e.g. KeyedScanLock in Images/Helper/), and use one static/singleton instance per scan kind (observed image, Docker instance, image version). The orchestrators are scoped, so the lock registries must be static or registered as singletons; a singleton injected via DI is easier to test.
• Runtime: wrap the body of ScanInstanceAsync (or the call at :1215-1219) with the instance lock, keyed by dockerInstance.Id.
• Vulnerability: wrap ApplyImageVulnerabilitiesAsync + the following SaveChangesAsync (VulnerabilityEnrichmentService.cs:449-464) with the image-version lock; guard DeactivateStaleFindingsAsync with a separate fixed-key lock. Do not hold a lock across the provider calls of a whole batch (that would serialize unrelated images and defeat MaxParallelScans). Acquire per-image locks one at a time (never two at once) to avoid deadlocks.
• Pitfall: the DbContext of the waiting run may hold stale tracked entities; load the existing findings after the lock is acquired (they are already loaded inside UpsertVulnerabilityFindingsAsync, keep it that way).
• Must not change: observed-image guard semantics; "duplicate scans are wasteful, never incorrect" across instances (guarantee in .squad/project.md); ExternalOperationResult handling; scan-run statuses and metrics.
• No configuration keys, no migration.

Related

• PR [Docs] Describe the current behavior by area #263 (area documents)
• F-021 (P4) — VulnerabilityFindings: unique index over nullable columns is ineffective #14 (VulnerabilityFindings unique index) — would turn the duplicate-insert race into a constraint violation; this issue removes the in-process race.
• F-032 (P5) — Shared scoped DbContext across the Blazor circuit; view-service lock does not cover write/scan paths #21 (shared scoped DbContext) — related concurrency concern in the UI scope.

Generated by Claude Code