Add malware advisory value object for finding construction

Bala Kumarrequested to merge
612089-malware-advisory-value-object into master

Summary

Adds Advisory.from_malware_advisory so both malicious-advisory consumer paths can build a finding from a pm_malware_advisories row, and fixes the severity those findings would have received.

Part of #612089 (closed). First of the Wave 0 foundations in &21156.

No behaviour changes on its own — nothing calls the new constructor until the follow-ups land.

Approach

Why a second constructor

Advisory.from_affected_package builds the value object that BuildFindingMapService turns into a finding, but it is shaped for public advisories: it reads solution_text off the affected package and carries CVSS vectors. Malware advisories have neither — the affected versions are malicious rather than vulnerable, so there is no fix to upgrade to and no CVSS to score.

The new constructor takes only the advisory. Nothing is read from the affected package, so accepting one would mislead. This is why it is not named from_malware_affected_package as the issue originally proposed.

Severity, and why it needed changing

FindingBuilder#severity derived severity solely from CVSS:

cvss_scores.first&.severity&.downcase

With no CVSS vectors that yields nil, and Enums::Vulnerability.parse_severity_level(nil) returns unknown. So every malware finding would have been reported as unknown severity.

Three options were considered:

  1. Synthesize a fixed CVSS vector so the existing derivation lands on critical.
  2. Let the advisory assert a severity that the builder prefers.
  3. Branch inside FindingBuilder on the advisory source.

Option 2 was chosen. Synthesizing a vector would put fabricated data into cvss_vectors_with_vendor and surface it in the UI as though the advisory carried a real score. Branching on source would couple a shared builder — used by public advisories and container scanning — to one specific source.

So Advisory takes an optional severity: defaulting to nil, and the builder became:

advisory.severity || cvss_scores.first&.severity&.downcase

Public advisories pass nil and keep deriving from CVSS exactly as before. A MALWARE_SEVERITY = 'critical' constant records why malware is not a graded risk: any affected version of a malicious package is fully compromised.

Fit with the closed decisions

Three decisions have since been settled, and none of them changes this MR — but one makes it matter more:

  • Keep both findings (#594791 (closed)). GLAM findings now sit alongside GLAD findings for the same package rather than being suppressed. A malware finding showing unknown next to a CVE finding showing high would read as a defect, so getting severity right is more visible under this decision, not less.
  • Reuse the 14-day publication window (#612096 (closed)). Affects which advisories trigger a CVS scan, not how a finding is built.
  • Not add-on gated (#612095). A licensing question, orthogonal to the value object.

The severity choice also stays compatible with keeping both findings: the value is asserted per advisory, so a GLAM finding is critical without altering how the GLAD finding beside it is scored.

Local testing

Steps, script, and before/after observations

Needs malware advisories in the local DB, which the malware advisory sync populates. The script reads a real synced advisory rather than a fixture, and writes nothing.

Save as /tmp/verify_severity.rb:

# Before/after for the malware advisory value object and its severity.
# Run on origin/master, then on 612089-malware-advisory-value-object.
XID = 'GLAM-2026-07-00002' # real synced GLAM advisory (base65-85x)

advisory = PackageMetadata::MalwareAdvisory.find_by(advisory_xid: XID)
abort "advisory #{XID} not in local DB — run the malware sync first" unless advisory

klass = Gitlab::VulnerabilityScanning::Advisory
puts "=== 1. does the value object have a malware constructor? ==="
if klass.respond_to?(:from_malware_advisory)
  puts '  Advisory.from_malware_advisory: DEFINED'
  vo = klass.from_malware_advisory(advisory: advisory)
else
  puts '  Advisory.from_malware_advisory: NOT DEFINED'
  puts '  -> callers must hand-roll the value object, or reuse from_affected_package,'
  puts '     which reads solution_text off an affected package malware advisories do not have'
  vo = nil
end

puts '=== 2. does it accept an asserted severity? ==='
args = { xid: advisory.advisory_xid, title: advisory.title, description: advisory.description,
         solution: nil, identifiers: advisory.identifiers, urls: advisory.urls,
         source_xid: advisory.source_xid }
begin
  probe = klass.new(**args, severity: 'critical')
  puts "  Advisory.new(severity:) accepted -> #{probe.severity.inspect}"
rescue ArgumentError => e
  puts "  Advisory.new(severity:) REJECTED -> #{e.message}"
end

puts '=== 3. severity the finding would receive ==='
vo ||= klass.new(**args)
cvss = [vo.cvss_v4, vo.cvss_v3, vo.cvss_v2].compact.select(&:valid?)
# mirrors FindingBuilder#severity on each branch
derived = if vo.respond_to?(:severity)
            vo.severity || cvss.first&.severity&.downcase
          else
            cvss.first&.severity&.downcase
          end
puts "  valid cvss vectors on a malware advisory: #{cvss.size}"
puts "  raw derived value:  #{derived.inspect}"
puts "  finding severity:   #{::Enums::Vulnerability.parse_severity_level(derived).inspect}"

puts '=== 4. GLAM identifier survives (drives malware_status downstream) ==='
puts "  first identifier: #{vo.identifiers.first['name'].inspect}"

Run on each side:

git checkout --detach origin/master
bundle exec rails runner /tmp/verify_severity.rb

git checkout 612089-malware-advisory-value-object
bundle exec rails runner /tmp/verify_severity.rb

Observations

Same advisory (GLAM-2026-07-00002, the base65-85x npm malware advisory) on origin/master versus this branch.

Check Before After
Advisory.from_malware_advisory not defined defined
Advisory.new(severity:) ArgumentError: unknown keyword: :severity accepted → "critical"
Valid CVSS vectors on the advisory 0 0
Raw derived severity nil "critical"
Severity the finding receives "unknown" "critical"
GLAM identifier preserved yes yes

The third row is the point: the advisory genuinely has no CVSS, so the old derivation had nothing to work from. The fix does not invent a score — it lets the source assert one.

The last row matters downstream: Sbom::Occurrence#malware_status and the Dependency REST entity derive "is malware" from the presence of a GLAM- identifier, so it has to survive the conversion.

Specs: 68 examples, 0 failures across advisory_spec.rb, both finding-builder specs, and build_finding_map_service_spec.rb. RuboCop clean.

Related

Edited by Bala Kumar

Merge request reports