Derive dependency list malware status from MalwareAdvisoriesFinder with a batched preloader

Summary

Derive the dependency list malware field from PackageMetadata::MalwareAdvisoriesFinder instead of from vulnerabilities carrying a GLAM- identifier, backed by a batched preloader so it does not become an N+1.

Motivation

Malware is a property of (purl_type, name, version). It does not depend on whether a particular project has run a security scan.

Deriving it from vulnerabilities instead makes it a property of an Sbom::Occurrence, and that is the sole reason the group-level implementation is as involved as it is. Sbom::AggregationsFinder collapses each component version to one representative occurrence via MIN(id), so vulnerabilities read off the representative miss GLAM findings recorded against the same component's other occurrences elsewhere in the group. Sbom::MalwareVulnerabilitiesPreloader exists only to work around that.

Keying on the package instead makes the aggregation problem disappear. There is one answer per component version, so there is nothing to reconcile across occurrences and the preloader becomes unnecessary.

It also covers a case the current approach cannot: flagging a malicious package when no security scan has run against the project.

Proposed approach

The query layer already supports batching. PackageMetadata::MalwareAffectedPackage.for_occurrences takes an array of components and builds a VALUES CTE joined on (purl_type, name):

scope :for_occurrences, ->(occurrences) do
  next none if occurrences.empty?
  # ... VALUES CTE, INNER JOIN on purl_type + name
end

MalwareAdvisoriesFinder passes a single-element array because its only consumer, Security::DependencyFirewall::FetchPackageMaliciousService, checks one package at a time at download time. A list surface needs the batched shape.

Both MalwareAffectedPackage and MalwareAdvisory are < ::SecApplicationRecord, the same database as Sbom::Occurrence, so there is no cross-database constraint.

Sketch:

  1. Collect (purl_type, name, version) for the page's component versions.
  2. One for_occurrences(components).with_advisory query.
  3. Match versions in Ruby using the existing Gitlab::VulnerabilityScanning::AdvisoryUtils matcher, as the finder already does.
  4. Assign the result through Sbom::Occurrence#malware_status=, the writer already used by Sbom::MalwareVulnerabilitiesPreloader.

Cost is comparable to the current implementation: one query per page plus a Ruby-side loop.

Open questions

These are the reason this is not a straight refactor. They need answering before implementation.

  1. It changes what the field asserts. Today malware: true means a scan produced a Vulnerability record that a user can click through to. Advisory-derived means the version appears in an advisory, whether or not anything was scanned. Decide which contract the badge should carry.
  2. Divergence from the vulnerability report. That surface uses has_glam_identifier? in ee/app/models/ee/vulnerability.rb. If the dependency list uses a different source, the two can disagree about the same package. Either both move or the inconsistency is accepted deliberately.
  3. No vulnerability lifecycle. Dismissal, resolution and state transitions have no equivalent on the advisory path. Note that malware_status_for does not filter on state today either, so this is worth settling for both paths at once.
  4. Withdrawn advisories. MalwareAdvisoriesFinder does not filter pm_malware_advisories.withdrawn_date. #612090 (closed) addresses this in the shared scopes and should land first.

Acceptance criteria

  • A batched preloader resolves malware status for a page of component versions in a bounded number of queries, verified by an N+1 spec.
  • Group aggregations return the correct status without the cross-occurrence lookup.
  • The semantics questions above are answered and recorded.
  • The REST, GraphQL and Elasticsearch surfaces agree on the same source of truth.

Related