[BUG] Over-long provider values abort the vulnerability refresh and leave remaining images unscanned #271

Summary

Vulnerability providers (Trivy, Docker Scout) return free-form strings that are written to length-limited columns without being shortened. If one value is longer than its column, for example a Trivy Title over 300 characters, PostgreSQL rejects the SaveChanges for that image version. The exception escapes the batch loop, so the whole vulnerability refresh ends Failed and every remaining image version goes unscanned in that run. Because the same advisory comes back on every run, the refresh then fails in the same place every time.

Area

• docs/areas/vulnerabilities.md (Findings, Limits and errors), added in [Docs] Describe the current behavior by area #263
• Security area (.squad/project.md): Database and migrations (only if column definitions change, which the recommended fix avoids). Otherwise none.

Current behavior

Column limits (verified in src/DockerUpdateGuard.Data/Configurations/VulnerabilityFindingConfiguration.cs:24-60 and ImageVersionConfiguration.cs:48-49):

Values copied as-is: the provider mapping copies the values straight through. See src/DockerUpdateGuard/Vulnerabilities/TrivyVulnerabilityProvider.cs:94-101 (Title, PkgName, InstalledVersion, FixedVersion, Class, Type) and DockerScoutVulnerabilityProvider.cs:127-134. They are written unchanged in src/DockerUpdateGuard/Images/VulnerabilityEnrichmentService.cs:

• UpdateFindingFromAdvisory (line 192 ff.) assigns Title, InstalledVersion, FixedVersion, PackageClass, PackageType.
• New findings get AdvisoryId and AffectedPackage unchanged (around line 259).
• The assessment message is the provider message unchanged (failureMessage, lines 310-318). One example is Trivy's "exited with code X: ".

No per-image isolation: in RunRefreshAsync (line 397 ff.), each image version is applied and then saved inside the batch loop (SaveChangesAsync at line 464). A DbUpdateException from that save leaves the loop and reaches the run-level catch (Exception exception) (line 485). That catch sets finalStatus = ScanRunStatus.Failed and records the message. Two things follow:

• the remaining image versions of the batch and all later batches are never scanned;
• the stale-findings pass of a full refresh is skipped;
• the change tracker still holds the rejected entities, so the final save of the scan run can fail as well. The retry at lines 503 ff. then clears the tracker to save the run.

Failure scenario: a full refresh covers 40 image versions. Trivy reports, for the 3rd image version, an advisory whose Title is 412 characters long, which happens with some GHSA/OSV advisories. Saving that image version fails with 22001: value too long for type character varying(300). Image versions 4–40 keep their old assessments, the run ends Failed, and the next scheduled run fails the same way.

Note: the SQLite and InMemory test databases do not enforce varchar(n) lengths, which is why no existing test catches this.

Expected behavior

1. Decision (product manager): provider values longer than their stored column are truncated to the column length, never rejected. Free-text values (title, assessment message) end with an ellipsis (…, U+2026) inside the limit. The result is at most the column length in characters, and a UTF-16 surrogate pair is never split.
2. Truncation happens before findings are deduplicated and matched (CreateFindingKey). Two advisories whose identities only differ beyond the limit then collapse into one finding by the existing dedup rule (highest severity, then highest CVSS), and the filtered unique index (ImageVersionId, AdvisoryId, AffectedPackage) WHERE IsActive cannot be violated by truncation.
3. Decision (product manager): a failure while applying or saving the results of one image version does not abort the refresh:
   • the pending changes of that image version are discarded;
   • its assessment is set to Failed with a message naming the save failure (itself limited to 2000 characters);
   • its existing findings stay untouched;
   • the message is added to the scan run;
   • the refresh continues with the remaining image versions;
   • the run ends Failed.
4. Cancellation is still propagated and never converted into a per-image failure.
5. The stale-findings pass of a full refresh still runs after a per-image failure. The failed image version counts as processed, so its findings are retained, as for a failed provider call today.

Acceptance criteria

• All values written to VulnerabilityFinding.AdvisoryId, Title, AffectedPackage, InstalledVersion, FixedVersion, PackageClass, PackageType and ImageVersion.VulnerabilityAssessmentMessage are limited to their column length.
• The limits are defined once, as constants used by both the EF configuration and the truncation code, so they cannot drift.
• Unit tests for the truncation helper (for example VulnerabilityValueLimiterTests, {TypeUnderTest}{Scenario}{ExpectedResult} naming):
  • shorter value unchanged;
  • value of exactly the limit unchanged;
  • longer value cut to the limit with …;
  • null stays null;
  • a surrogate pair at the boundary is not split.
• VulnerabilityEnrichmentServiceTests: VulnerabilityEnrichmentServiceRefreshAsyncTruncatesOverlongAdvisoryValuesAsync. The stored finding's title, package, versions and class/type are no longer than their limits, and the title ends with ….
• VulnerabilityEnrichmentServiceTests: VulnerabilityEnrichmentServiceRefreshAsyncAdvisoriesDifferingOnlyBeyondLimitAreDeduplicatedAsync. One active finding results.
• VulnerabilityEnrichmentServiceTests: VulnerabilityEnrichmentServiceRefreshAsyncSaveFailureForOneImageContinuesWithRemainingImagesAsync:
  • use SqliteTestDatabase.CreateSaveChangesFailingDbContext() to fail the save of the first image version of a multi-image (and multi-batch) refresh;
  • the later image versions get their assessments and findings;
  • the failed one is Failed and its old findings are still active;
  • the run is Failed and has the message.
• VulnerabilityEnrichmentServiceTests: a long provider failure message is stored truncated to 2000 characters.
• Existing tests stay green, in particular …ProviderExceptionMarksRunFailedAsync and …TerminalSaveFailureIsLoggedAndRetriedAsync.
• docs/areas/vulnerabilities.md describes truncation and per-image failure isolation under Findings / Limits and errors, and its Known gaps / open-question entry for this issue is removed.
• Format, build, tests, analyzer gate and coverage gate pass (.squad/stack.md).

Implementation notes

• Where to truncate: centrally in VulnerabilityEnrichmentService, recommended as a normalization step applied to the advisory list before DeduplicateAdvisories, plus at the two assessment-message assignments. This covers both providers and any future one. Truncating inside each provider is the alternative, but it duplicates the rule and misses the assessment message.
• Limits as constants: add for example public const int TitleMaxLength = 300; etc. on a small static class in DockerUpdateGuard.Data (or on the entities). VulnerabilityFindingConfiguration / ImageVersionConfiguration use them in HasMaxLength(...). The values do not change, so no migration is generated. Verify with dotnet ef migrations add producing an empty diff, then discard it. If a migration were ever needed, the naming is UpdateN per AGENTS.md.
• Per-image isolation: wrap "apply results + SaveChangesAsync" for one image version in try/catch (Exception) when (exception is not OperationCanceledException || cancellationToken.IsCancellationRequested == false).
  • On failure, revert only that image version's tracked changes: Added findings → Detached; Modified findings and the image version → reset to original values (entry.CurrentValues.SetValues(entry.OriginalValues); entry.State = EntityState.Unchanged). Then set the Failed assessment and save again.
  • Do not call ChangeTracker.Clear() here. It would also detach the scan run and the image versions of later batches, which were loaded with tracking in RefreshAsync.
• Logging: log the per-image failure with the image reference only. Provider values must not end up in log templates unsanitized (security area Logging and telemetry of external data).
• Must not change:
  • the run status rules for provider results;
  • failed assessments leave findings untouched;
  • the guarantee that integration clients never throw for expected failures (.squad/project.md);
  • the one-active-finding-per-key invariant.
• No configuration or README impact.

Related

• [Docs] Describe the current behavior by area #263 (area documents)
• F-048 (P7) — Scan-resilience defects (F-024 batch abort, F-025 transient finding loss) without regression test #19: the same batch-abort pattern in the scan orchestrators. This issue fixes it for the vulnerability refresh; F-048 (P7) — Scan-resilience defects (F-024 batch abort, F-025 transient finding loss) without regression test #19 remains for the orchestrators.
• F-021 (P4) — VulnerabilityFindings: unique index over nullable columns is ineffective #14 (VulnerabilityFindings unique index), F-015 (P2) — Findings lose package granularity; the planned unique index is ineffective #3 (findings lose package granularity), F-050 (P7) — Test DB fidelity: SQLite/InMemory instead of PostgreSQL hides F-021 and provides deceptive F-032 safety #28 (test DB fidelity: SQLite does not enforce varchar(n))

Generated by Claude Code