Invalidate cached SBOM scan results on a malware advisory sync

Summary

Makes a malware advisory sync invalidate cached SBOM scan results, so a pipeline is not served a result computed before the advisory arrived.

Part of #612092 (closed), in &21156.

Approach

SbomScanResultCachingService reuses a previous scan result when the project and sbom_digest match and no advisory sync has happened since. That last check looked at one data type:

PackageMetadata::Checkpoint
  .for_advisories          # data_type = advisories only
  .with_purl_types(purl_types)
  .synced_since(timestamp.to_i)
  .any?

Malware checkpoints are data_type = malware_advisories, so a sync that advanced only those invalidated nothing. Once the SBOM scan path emits malware findings (!249853 (merged)), a project whose dependency set is unchanged is served a cached result that predates the malware advisory and silently omits it. Nothing errors, and a cache hit needs an identical sbom_digest — which is exactly the ordinary "re-run a pipeline without touching dependencies" case.

Why a scope, and why this name

"Has an advisory sync happened since" is one question about two data types, not two questions, so it belongs in a scope rather than two .any? calls in the service.

The name encodes the criterion rather than the membership. for_all_advisories would be accurate today, since advisories and malware_advisories are the only advisory data types — but it would silently absorb a future advisory-shaped type that produces no findings, and the caching service would start over-invalidating with nothing failing. for_finding_advisories names the property the caching decision actually depends on.

licenses and cve_enrichment stay out because neither produces findings, so a licenses sync should still allow reuse. A spec asserts that, so the scope cannot quietly widen.

How large the problem is

Bounded, not indefinite. SbomScan::EXPIRED_AGE is 2 days and DestroyExpiredSbomScansWorker prunes older records, so the worst case is a couple of days of a project reporting no malware finding for an advisory already in the database.

Worth noting separately: find_cached_scan does not apply the not_expired scope, so reuse is bounded by when the pruning worker runs rather than by EXPIRED_AGE itself. That is pre-existing and unrelated to malware, and is not changed here.

Test coverage

  • A malware-only sync newer than the cached scan, with a matching purl_type, returns :gone.
  • The same with no matching purl_type still allows reuse, so with_purl_types still applies.
  • A malware sync older than the cached scan allows reuse.
  • A licenses sync newer than the cached scan still allows reuse, confirming the scope does not over-invalidate.
  • for_finding_advisories returns both advisory kinds and excludes licenses and cve_enrichment.

78 examples, 0 failures across checkpoint_spec.rb and sbom_scan_result_caching_service_spec.rb. RuboCop clean.

Database

No schema change. One query is modified: the freshness check in advisories_synced_since?. .any? compiles to exists?, so the runtime statement is SELECT 1 … LIMIT 1 rather than SELECT *.

-- before
WHERE data_type = 1        AND purl_type IN () AND sequence >= <epoch>
-- after
WHERE data_type IN (1, 4)  AND purl_type IN () AND sequence >= <epoch>

1 is advisories and 4 is malware_advisories. The predicates are enum integers plus an epoch-second cursor, so no IDs are involved and these run in Database Lab as written.

The table cannot grow with usage. pm_checkpoints is keyed by (purl_type, data_type, version_format), so its cardinality is bounded by those enums — a ceiling of roughly 200 rows — rather than by projects, advisories or scans. db/docs/pm_checkpoints.yml declares table_size: small. A sequential scan is therefore the expected plan, and the change widens one equality into a two-element IN on a table that stays trivially small.

Index available, from db/structure.sql:

CREATE UNIQUE INDEX pm_checkpoints_path_components
  ON pm_checkpoints USING btree (purl_type, data_type, version_format);

purl_type and data_type are both covered by it; sequence is a filter only. No index change is proposed, for the cardinality reason above.

Queries and query plans

The modified query, before and after

-- AFTER (this MR)
EXPLAIN (ANALYZE, BUFFERS)
SELECT 1 AS one FROM pm_checkpoints
WHERE data_type IN (1, 4) AND purl_type IN (1, 4, 6, 8) AND sequence >= 1700000000
LIMIT 1;

postgres.ai plan:

-- BEFORE
EXPLAIN (ANALYZE, BUFFERS)
SELECT 1 AS one FROM pm_checkpoints
WHERE data_type = 1 AND purl_type IN (1, 4, 6, 8) AND sequence >= 1700000000
LIMIT 1;

Before: https://console.postgres.ai/gitlab/projects/gitlab-production-main/sessions/54924/commands/158036

After: https://console.postgres.ai/gitlab/projects/gitlab-production-main/sessions/54922/commands/158035

Uses the same Index Scan using pm_checkpoints_path_components on public.pm_checkpoints.

Local testing

Steps, script, and before/after observations

Drives the real service. Picks a purl type that has no checkpoints at all, so the npm and pypi sync cursors are never touched, and removes everything it creates. It reuses an existing project and build rather than creating any.

Save as /tmp/verify_cache.rb:

# Before/after for SBOM scan-result cache invalidation on a malware advisory sync.
# Run on origin/master, then on 612092-invalidate-sbom-cache-on-malware-sync.
DIGEST = 'sha256v1-cache-invalidation-check'

# Use a purl_type that has no checkpoints at all, so the real npm/pypi sync cursors
# are never touched. Everything created here is removed at the end.
used = PackageMetadata::Checkpoint.distinct.pluck(:purl_type)
purl = (::Enums::Sbom.purl_types.keys - used - ['not_provided']).first or abort 'no unused purl_type available'
purl_int = ::Enums::Sbom.purl_types[purl]
puts "using unused purl_type: #{purl} (#{purl_int})"

svc = Security::VulnerabilityScanning::SbomScanResultCachingService

def run(svc, build, digest, purl_int)
  svc.new(build).execute(digest, [purl_int]).payload[:status]
end

# Reuse the existing verification fixture project and one of its builds, so nothing
# new is created beyond the scan record and the throwaway checkpoints.
project = Project.find_by_full_path('bala-test-group/malware-sbom-verification') or
  abort 'fixture project not found'
build = Ci::Build.where(project: project).last or abort 'no build on the fixture project'
scan_at = 1.hour.ago
# A finished scan for this project+digest is all the caching service looks for.
# Re-created before every case: a successful reuse inserts another finished scan
# dated now, which would otherwise become the cached one for the next case.
def reset_scan!(project, build, digest, scan_at)
  Security::VulnerabilityScanning::SbomScan.where(sbom_digest: digest).destroy_all
  scan = Security::VulnerabilityScanning::SbomScan.new(
    project: project, build: build, sbom_digest: digest, status: 2)
  scan.save!(validate: false)
  scan.update_columns(created_at: scan_at, updated_at: scan_at)
end

fresh = scan_at.to_i + 60 # a sync that happened after the cached scan
created = []

begin
  reset_scan!(project, build, DIGEST, scan_at)
  puts '=== 1. no advisory sync since the cached scan (baseline) ==='
  puts "  -> #{run(svc, build, DIGEST, purl_int).inspect}  (expected :created, cache reused)"

  reset_scan!(project, build, DIGEST, scan_at)
  puts '=== 2. only a MALWARE advisory sync since the cached scan ==='
  created << PackageMetadata::Checkpoint.create!(data_type: 'malware_advisories', version_format: 'v3',
    purl_type: purl, sequence: fresh, chunk: 0)
  status = run(svc, build, DIGEST, purl_int)
  puts "  -> #{status.inspect}"
  puts(status == :gone ? '     cache INVALIDATED (correct)' : '     cache REUSED -- stale result served, malware finding missing')

  reset_scan!(project, build, DIGEST, scan_at)
  puts '=== 3. control: only a LICENSES sync since the cached scan ==='
  created.each(&:destroy!)
  created = [PackageMetadata::Checkpoint.create!(data_type: 'licenses', version_format: 'v2',
    purl_type: purl, sequence: fresh, chunk: 0)]
  status = run(svc, build, DIGEST, purl_int)
  puts "  -> #{status.inspect}"
  puts(status == :created ? '     cache reused (correct -- licenses produce no findings)' : '     over-invalidated')

  reset_scan!(project, build, DIGEST, scan_at)
  puts '=== 4. control: a PUBLIC advisory sync since the cached scan ==='
  created.each(&:destroy!)
  created = [PackageMetadata::Checkpoint.create!(data_type: 'advisories', version_format: 'v2',
    purl_type: purl, sequence: fresh, chunk: 0)]
  puts "  -> #{run(svc, build, DIGEST, purl_int).inspect}  (expected :gone on both branches)"
ensure
  created.each { |c| c.destroy! rescue nil }
  Security::VulnerabilityScanning::SbomScan.where(sbom_digest: DIGEST).destroy_all
  puts '=== cleaned up ==='
end

Run on each side:

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

git checkout 612092-invalidate-sbom-cache-on-malware-sync
bundle exec rails runner /tmp/verify_cache.rb

Observations

Case Before After
No advisory sync since the cached scan :created :created
Only a malware advisory sync since :created — stale result served :gone — invalidated
Only a licenses sync since :created :created
A public advisory sync since :gone :gone

Only the second row changes, which is the point.

The fourth row is the control worth reading: a public advisory sync already returns :gone on both sides, so the invalidation mechanism itself works. That is what makes the second row a malware-specific gap rather than a broken harness. The third row is the opposite control — licenses produce no findings and must keep allowing reuse, so it must not change either.

One note if you run this yourself: a successful reuse inserts another finished scan dated now, which then becomes the cached one. The script re-creates the fixture scan before each case for that reason; without it every case after the first returns :created and the result looks meaningless.

Edited by Bala Kumar

Merge request reports

Loading
Loading