From 5f3523f4b4599b12980de74572c991dd99cb1faa Mon Sep 17 00:00:00 2001 From: Dave Shrewsberry Date: Mon, 6 Apr 2026 12:10:59 -0400 Subject: [PATCH 1/4] X-Smart-Branch-Parent: master From 271fdf4f2e4258fd827a08bf52b07ae3e3675388 Mon Sep 17 00:00:00 2001 From: Dave Shrewsberry Date: Mon, 6 Apr 2026 12:23:07 -0400 Subject: [PATCH 2/4] fix: restore component risk score population in ranker PR #17422 removed UpsertRisk calls from reprocessImageComponentRisk and reprocessNodeComponentRisk because the risk table rows were never read. However, UpsertRisk had a side effect: it called ranker.Add(), which populated the in-memory component ranker. Downstream, updateComponentRisk in the image/node datastores reads from this ranker before every DB write, overwriting component.RiskScore with the ranker value (0 for missing entries). This caused image_component_v2.riskscore to always be 0. Fix: call ranker.Add() directly after calculating the risk score. Tests: add regression tests verifying the ranker is updated. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) --- central/risk/manager/manager.go | 14 ++++- central/risk/manager/manager_test.go | 91 ++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 2 deletions(-) diff --git a/central/risk/manager/manager.go b/central/risk/manager/manager.go index 9d071d452f64a..052cda4ea5169 100644 --- a/central/risk/manager/manager.go +++ b/central/risk/manager/manager.go @@ -356,7 +356,12 @@ func (e *managerImpl) reprocessImageComponentRisk(imageComponent *storage.Embedd } imageComponent.RiskScore = risk.GetScore() - // skip direct upsert here since it is handled during image upsert + // Update the in-memory ranker so that downstream updateComponentRisk + // in the imagev2 datastore reads the correct score before DB write. + e.imageComponentRanker.Add( + scancomponent.ComponentIDV2(imageComponent, imageID, componentIndex), + risk.GetScore(), + ) } // reprocessNodeComponentRisk will reprocess risk of node components and save the results. @@ -370,7 +375,12 @@ func (e *managerImpl) reprocessNodeComponentRisk(nodeComponent *storage.Embedded } nodeComponent.RiskScore = risk.GetScore() - // skip direct upsert here since it is handled during node upsert + // Update the in-memory ranker so that downstream updateComponentRisk + // in the node datastore reads the correct score before DB write. + e.nodeComponentRanker.Add( + scancomponent.ComponentID(nodeComponent.GetName(), nodeComponent.GetVersion(), os), + risk.GetScore(), + ) } func (e *managerImpl) updateNamespaceRisk(nsID string, oldDeploymentScore float32, newDeploymentScore float32) { diff --git a/central/risk/manager/manager_test.go b/central/risk/manager/manager_test.go index ca2f373bfe91f..e70fbcbb83c8b 100644 --- a/central/risk/manager/manager_test.go +++ b/central/risk/manager/manager_test.go @@ -1,6 +1,7 @@ package manager import ( + "context" "fmt" "testing" @@ -18,6 +19,7 @@ import ( "github.com/stackrox/rox/pkg/features" "github.com/stackrox/rox/pkg/images/integration" integrationSetMocks "github.com/stackrox/rox/pkg/images/integration/mocks" + "github.com/stackrox/rox/pkg/scancomponent" scannerSetMocks "github.com/stackrox/rox/pkg/scanners/mocks" "github.com/stackrox/rox/pkg/scanners/types" scannerTypesMocks "github.com/stackrox/rox/pkg/scanners/types/mocks" @@ -386,3 +388,92 @@ func TestReprocessDeploymentRiskUsesCorrectImageID(t *testing.T) { }) } } + +// stubImageComponentScorer returns a fixed risk score for any component. +type stubImageComponentScorer struct { + score float32 +} + +func (s *stubImageComponentScorer) Score(_ context.Context, sc scancomponent.ScanComponent, _ string, imageComponent *storage.EmbeddedImageScanComponent, imageID string, index int) *storage.Risk { + componentID := scancomponent.ComponentIDV2(imageComponent, imageID, index) + return &storage.Risk{ + Score: s.score, + Results: []*storage.Risk_Result{ + {Name: "stub", Score: s.score}, + }, + Subject: &storage.RiskSubject{ + Id: componentID, + Type: storage.RiskSubjectType_IMAGE_COMPONENT, + }, + } +} + +// stubNodeComponentScorer returns a fixed risk score for any component. +type stubNodeComponentScorer struct { + score float32 +} + +func (s *stubNodeComponentScorer) Score(_ context.Context, _ scancomponent.ScanComponent, _ string) *storage.Risk { + return &storage.Risk{ + Score: s.score, + Results: []*storage.Risk_Result{ + {Name: "stub", Score: s.score}, + }, + Subject: &storage.RiskSubject{ + Type: storage.RiskSubjectType_NODE_COMPONENT, + }, + } +} + +func TestReprocessImageComponentRiskUpdatesRanker(t *testing.T) { + ranker := ranking.NewRanker() + mgr := &managerImpl{ + imageComponentScorer: &stubImageComponentScorer{score: 3.5}, + imageComponentRanker: ranker, + } + + component := &storage.EmbeddedImageScanComponent{ + Name: "openssl", + Version: "1.1.1", + Vulns: []*storage.EmbeddedVulnerability{ + {Cve: "CVE-2021-1234", Cvss: 7.5}, + }, + } + imageID := "sha256:abc123" + componentIndex := 0 + + mgr.reprocessImageComponentRisk(component, "linux", imageID, componentIndex) + + assert.InDelta(t, 3.5, component.RiskScore, 0.001, "component RiskScore should be set") + + expectedID := scancomponent.ComponentIDV2(component, imageID, componentIndex) + rankerScore := ranker.GetScoreForID(expectedID) + assert.InDelta(t, 3.5, rankerScore, 0.001, + "ranker must be updated with the calculated score so downstream updateComponentRisk does not clobber it") +} + +func TestReprocessNodeComponentRiskUpdatesRanker(t *testing.T) { + ranker := ranking.NewRanker() + mgr := &managerImpl{ + nodeComponentScorer: &stubNodeComponentScorer{score: 2.8}, + nodeComponentRanker: ranker, + } + + component := &storage.EmbeddedNodeScanComponent{ + Name: "kernel", + Version: "5.4.0", + Vulns: []*storage.EmbeddedVulnerability{ + {Cve: "CVE-2022-5678", Cvss: 6.0}, + }, + } + os := "linux" + + mgr.reprocessNodeComponentRisk(component, os) + + assert.InDelta(t, 2.8, component.RiskScore, 0.001, "component RiskScore should be set") + + expectedID := scancomponent.ComponentID(component.GetName(), component.GetVersion(), os) + rankerScore := ranker.GetScoreForID(expectedID) + assert.InDelta(t, 2.8, rankerScore, 0.001, + "ranker must be updated with the calculated score so downstream updateComponentRisk does not clobber it") +} From 5bb213b43462d238872515dbae79f1ddd692fedd Mon Sep 17 00:00:00 2001 From: Dave Shrewsberry Date: Mon, 6 Apr 2026 13:10:00 -0400 Subject: [PATCH 3/4] test: add sql_integration tests for component risk score persistence Add TestComponentRiskScorePersistedToDB to both v1 and v2 image datastore test suites. These tests pre-populate the component ranker with known scores, upsert an image, then read back from DB to verify the scores survive through updateComponentRisk and into the database. Partially generated by AI. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../datastore_impl_flat_postgres_test.go | 41 +++++++++++++++++++ .../datastoretest/datastore_impl_test.go | 41 +++++++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/central/image/datastore/datastore_impl_flat_postgres_test.go b/central/image/datastore/datastore_impl_flat_postgres_test.go index 38acd1643b674..d4414bddf30c6 100644 --- a/central/image/datastore/datastore_impl_flat_postgres_test.go +++ b/central/image/datastore/datastore_impl_flat_postgres_test.go @@ -963,6 +963,47 @@ func getTestImage(id string) *storage.Image { } } +// TestComponentRiskScorePersistedToDB verifies that when the component ranker +// contains a risk score for a component, UpsertImage writes that score to the +// database. This is a regression test for the bug where the ranker was not +// being updated after risk calculation, causing updateComponentRisk to +// overwrite calculated scores with 0. +func (s *ImageFlatPostgresDataStoreTestSuite) TestComponentRiskScorePersistedToDB() { + ctx := sac.WithAllAccess(context.Background()) + + componentRanker := ranking.NewRanker() + ds := NewWithPostgres( + pgStoreV2.New(s.db, false, keyfence.ImageKeyFenceSingleton()), + s.mockRisk, + ranking.NewRanker(), + componentRanker, + ) + + img := fixtures.GetImageWithUniqueComponents(3) + + // Pre-populate the ranker with known scores for each component, + // simulating what the risk manager does after calculating risk. + expectedScores := []float32{1.5, 2.7, 4.2} + for index, component := range img.GetScan().GetComponents() { + componentID := scancomponent.ComponentIDV2(component, img.GetId(), index) + componentRanker.Add(componentID, expectedScores[index]) + } + + s.NoError(ds.UpsertImage(ctx, img)) + + // Read back from DB — the risk scores should match what was in the ranker. + got, found, err := ds.GetImage(ctx, img.GetId()) + s.NoError(err) + s.True(found) + + gotComponents := got.GetScan().GetComponents() + s.Require().Len(gotComponents, 3) + for i, component := range gotComponents { + s.InDelta(expectedScores[i], component.GetRiskScore(), 0.001, + "component %q risk score should be persisted to DB", component.GetName()) + } +} + func cloneAndUpdateRiskPriority(image *storage.Image) *storage.Image { cloned := image.CloneVT() cloned.Priority = 1 diff --git a/central/imagev2/datastoretest/datastore_impl_test.go b/central/imagev2/datastoretest/datastore_impl_test.go index 33907a1ff77e5..e891328d0efcd 100644 --- a/central/imagev2/datastoretest/datastore_impl_test.go +++ b/central/imagev2/datastoretest/datastore_impl_test.go @@ -1152,3 +1152,44 @@ func (s *ImageV2DataStoreTestSuite) TestSearchListImages() { s.True(imageDigests.Contains(img2.GetDigest())) s.True(imageDigests.Contains(img3.GetDigest())) } + +// TestComponentRiskScorePersistedToDB verifies that when the component ranker +// contains a risk score for a component, UpsertImage writes that score to the +// database. This is a regression test for the bug where the ranker was not +// being updated after risk calculation, causing updateComponentRisk to +// overwrite calculated scores with 0. +func (s *ImageV2DataStoreTestSuite) TestComponentRiskScorePersistedToDB() { + ctx := sac.WithAllAccess(context.Background()) + + componentRanker := ranking.NewRanker() + ds := imageDataStoreV2.NewWithPostgres( + pgStore.New(s.testDB.DB, false, keyfence.ImageKeyFenceSingleton()), + s.mockRisk, + ranking.NewRanker(), + componentRanker, + ) + + img := fixtures.GetImageV2WithUniqueComponents(3) + + // Pre-populate the ranker with known scores for each component, + // simulating what the risk manager does after calculating risk. + expectedScores := []float32{1.5, 2.7, 4.2} + for index, component := range img.GetScan().GetComponents() { + componentID := scancomponent.ComponentIDV2(component, img.GetId(), index) + componentRanker.Add(componentID, expectedScores[index]) + } + + s.NoError(ds.UpsertImage(ctx, img)) + + // Read back from DB — the risk scores should match what was in the ranker. + got, found, err := ds.GetImage(ctx, img.GetId()) + s.NoError(err) + s.True(found) + + gotComponents := got.GetScan().GetComponents() + s.Require().Len(gotComponents, 3) + for i, component := range gotComponents { + s.InDelta(expectedScores[i], component.GetRiskScore(), 0.001, + "component %q risk score should be persisted to DB", component.GetName()) + } +} From 2fcf3441a8ff2473c6d5b4798d265ad7d843a034 Mon Sep 17 00:00:00 2001 From: Dave Shrewsberry Date: Mon, 6 Apr 2026 13:10:06 -0400 Subject: [PATCH 4/4] fix: use proto getter for RiskScore in tests to satisfy protogetter lint Co-Authored-By: Claude Opus 4.6 (1M context) --- central/risk/manager/manager_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/central/risk/manager/manager_test.go b/central/risk/manager/manager_test.go index e70fbcbb83c8b..e446292908034 100644 --- a/central/risk/manager/manager_test.go +++ b/central/risk/manager/manager_test.go @@ -444,7 +444,7 @@ func TestReprocessImageComponentRiskUpdatesRanker(t *testing.T) { mgr.reprocessImageComponentRisk(component, "linux", imageID, componentIndex) - assert.InDelta(t, 3.5, component.RiskScore, 0.001, "component RiskScore should be set") + assert.InDelta(t, 3.5, component.GetRiskScore(), 0.001, "component RiskScore should be set") expectedID := scancomponent.ComponentIDV2(component, imageID, componentIndex) rankerScore := ranker.GetScoreForID(expectedID) @@ -470,7 +470,7 @@ func TestReprocessNodeComponentRiskUpdatesRanker(t *testing.T) { mgr.reprocessNodeComponentRisk(component, os) - assert.InDelta(t, 2.8, component.RiskScore, 0.001, "component RiskScore should be set") + assert.InDelta(t, 2.8, component.GetRiskScore(), 0.001, "component RiskScore should be set") expectedID := scancomponent.ComponentID(component.GetName(), component.GetVersion(), os) rankerScore := ranker.GetScoreForID(expectedID)