From 6f820be9aa6b2825aec222a25ecc85a6d1affd85 Mon Sep 17 00:00:00 2001 From: Saif Chaudhry Date: Fri, 2 Oct 2026 09:24:59 -0700 Subject: [PATCH] fix(policy): preserve deploy checks with image exclusions Backport PR #23107 to release-5.0 so secured clusters on this stream regain deploy-time checks. Keep the behavior notice in the next-release changelog for the release engineer to place in the appropriate patch. (cherry picked from commit 7fdaba163cc0cb24f009a70ba8ff0bd7d90cb56a) Signed-off-by: Saif Chaudhry --- CHANGELOG.md | 6 + central/policy/matcher/cluster.go | 4 + central/policy/matcher/deployment.go | 4 + central/policy/matcher/matcher.go | 7 + central/policy/matcher/matcher_test.go | 38 +++++ central/policy/matcher/namespace.go | 2 +- pkg/detection/compiled_policy.go | 11 +- pkg/detection/compiled_policy_test.go | 11 ++ .../deploytime/detector_impl_test.go | 138 ++++++++++++++++++ pkg/detection/exclusion_test.go | 22 +++ 10 files changed, 240 insertions(+), 3 deletions(-) create mode 100644 central/policy/matcher/matcher_test.go create mode 100644 pkg/detection/deploytime/detector_impl_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 687cb051bba72..562d435a64d53 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,12 @@ Put an entry in this file if your change is user-visible and you consider it _pa Changes should still be described appropriately in JIRA/doc input pages, for inclusion in downstream release notes. +## [NEXT RELEASE] + +### Technical Changes + +- ROX-37315: Image exclusions no longer turn off deploy-time checks. Previously, an image exclusion on a policy with Build and Deploy stages stopped that policy from raising deploy-time violations or blocking any deployment. Image exclusions now apply only at Build, so after upgrading, affected policies raise deploy-time violations again and block deployments if enforcement is on. To skip apps at deploy time, use deployment exclusions. + ## [5.0.0] ### Added Features diff --git a/central/policy/matcher/cluster.go b/central/policy/matcher/cluster.go index b1b57f1bf151a..623a74c2b7a5c 100644 --- a/central/policy/matcher/cluster.go +++ b/central/policy/matcher/cluster.go @@ -48,6 +48,10 @@ func (m *clusterMatcher) anyExclusionMatches(exclusions []*storage.Exclusion) bo } func (m *clusterMatcher) exclusionMatches(exclusion *storage.Exclusion) bool { + if !appliesToDeployments(exclusion) { + return false + } + cs, err := scopecomp.CompileScope(exclusion.GetDeployment().GetScope(), nil, nil) if err != nil { utils.Should(errors.Wrap(err, "could not compile excluded scopes")) diff --git a/central/policy/matcher/deployment.go b/central/policy/matcher/deployment.go index 6058438bc79e8..2098b4090e8ce 100644 --- a/central/policy/matcher/deployment.go +++ b/central/policy/matcher/deployment.go @@ -56,6 +56,10 @@ func (m *deploymentMatcher) anyExclusionMatches(ctx context.Context, exclusions } func (m *deploymentMatcher) exclusionMatches(ctx context.Context, exclusion *storage.Exclusion) bool { + if !appliesToDeployments(exclusion) { + return false + } + // If excluded scope does not match the deployment then no need to check for deployment name if !m.scopeMatches(ctx, exclusion.GetDeployment().GetScope()) { return false diff --git a/central/policy/matcher/matcher.go b/central/policy/matcher/matcher.go index a80cd6c64ad54..4ecf17b5645f1 100644 --- a/central/policy/matcher/matcher.go +++ b/central/policy/matcher/matcher.go @@ -11,3 +11,10 @@ type Matcher interface { FilterApplicablePolicies(ctx context.Context, policies []*storage.Policy) (applicable []*storage.Policy, notApplicable []*storage.Policy) IsPolicyApplicable(ctx context.Context, policy *storage.Policy) bool } + +// appliesToDeployments reports whether the exclusion has a deployment part. Image-only exclusions +// are applied to images, so they must not exclude deployments, namespaces, or clusters. Without this +// check, the nil deployment scope matches everything and the policy looks inapplicable everywhere. +func appliesToDeployments(exclusion *storage.Exclusion) bool { + return exclusion.GetDeployment() != nil +} diff --git a/central/policy/matcher/matcher_test.go b/central/policy/matcher/matcher_test.go new file mode 100644 index 0000000000000..f142435bbbc00 --- /dev/null +++ b/central/policy/matcher/matcher_test.go @@ -0,0 +1,38 @@ +package matcher + +import ( + "context" + "testing" + + "github.com/stackrox/rox/generated/storage" + "github.com/stretchr/testify/assert" +) + +// TestImageOnlyExclusionDoesNotExcludeEntities checks that an image-only exclusion leaves the policy +// applicable to every deployment, namespace, and cluster, while a deployment exclusion still excludes. +func TestImageOnlyExclusionDoesNotExcludeEntities(t *testing.T) { + ctx := context.Background() + deployment := &storage.Deployment{Name: "payments", ClusterId: "cluster1", Namespace: "shop"} + namespace := &storage.NamespaceMetadata{Name: "shop", ClusterId: "cluster1"} + cluster := &storage.Cluster{Id: "cluster1"} + + matchers := map[string]Matcher{ + "deployment": NewDeploymentMatcher(deployment, nil, nil), + "namespace": NewNamespaceMatcher(namespace), + "cluster": NewClusterMatcher(cluster, []*storage.NamespaceMetadata{namespace}), + } + + imageOnly := &storage.Policy{Exclusions: []*storage.Exclusion{ + {Image: &storage.Exclusion_Image{Name: "docker.io/library/nginx"}}, + }} + clusterExclusion := &storage.Policy{Exclusions: []*storage.Exclusion{ + {Deployment: &storage.Exclusion_Deployment{Scope: &storage.Scope{Cluster: "cluster1"}}}, + }} + + for name, m := range matchers { + t.Run(name, func(t *testing.T) { + assert.True(t, m.IsPolicyApplicable(ctx, imageOnly), "image-only exclusion should not exclude") + assert.False(t, m.IsPolicyApplicable(ctx, clusterExclusion), "cluster-scoped exclusion should exclude") + }) + } +} diff --git a/central/policy/matcher/namespace.go b/central/policy/matcher/namespace.go index 1cf53532e9f72..4d4f182056dde 100644 --- a/central/policy/matcher/namespace.go +++ b/central/policy/matcher/namespace.go @@ -46,7 +46,7 @@ func (m *namespaceMatcher) anyExclusionMatches(exclusions []*storage.Exclusion) } func (m *namespaceMatcher) exclusionMatches(exclusion *storage.Exclusion) bool { - return m.scopeMatches(exclusion.GetDeployment().GetScope()) + return appliesToDeployments(exclusion) && m.scopeMatches(exclusion.GetDeployment().GetScope()) } func (m *namespaceMatcher) anyScopeMatches(scopes []*storage.Scope) bool { diff --git a/pkg/detection/compiled_policy.go b/pkg/detection/compiled_policy.go index 9fedc810495f5..5f64900b4f543 100644 --- a/pkg/detection/compiled_policy.go +++ b/pkg/detection/compiled_policy.go @@ -514,8 +514,15 @@ func newCompiledExclusion(exclusion *storage.Exclusion) (*compiledExclusion, err return cx, nil } +// appliesToDeployments reports whether the exclusion has a deployment part. Image-only exclusions are +// applied to images, so they must not match deployments or audit events. Without this check, the nil +// deployment scope matches everything and the exclusion turns the policy off for every deployment. +func (cw *compiledExclusion) appliesToDeployments() bool { + return cw.exclusion.GetDeployment() != nil +} + func (cw *compiledExclusion) MatchesDeployment(ctx context.Context, deployment *storage.Deployment) bool { - if exclusionIsExpired(cw.exclusion) { + if !cw.appliesToDeployments() || exclusionIsExpired(cw.exclusion) { return false } @@ -527,7 +534,7 @@ func (cw *compiledExclusion) MatchesDeployment(ctx context.Context, deployment * } func (cw *compiledExclusion) MatchesAuditEvent(ctx context.Context, auditEvent *storage.KubernetesEvent) bool { - if exclusionIsExpired(cw.exclusion) { + if !cw.appliesToDeployments() || exclusionIsExpired(cw.exclusion) { return false } if !cw.cs.MatchesAuditEvent(ctx, auditEvent) { diff --git a/pkg/detection/compiled_policy_test.go b/pkg/detection/compiled_policy_test.go index db16cf30375cf..8b61fc306e9b6 100644 --- a/pkg/detection/compiled_policy_test.go +++ b/pkg/detection/compiled_policy_test.go @@ -68,6 +68,17 @@ func TestCompiledPolicyScopesAndExclusions(t *testing.T) { exclusions: []*storage.Exclusion{{Deployment: &storage.Exclusion_Deployment{Scope: appStackRoxScope}}}, shouldApplyTo: []*storage.Deployment{stackRoxNSDep}, }, + { + desc: "image-only exclusion does not exclude any deployment", + exclusions: []*storage.Exclusion{{Image: &storage.Exclusion_Image{Name: "docker.io/library/unrelated"}}}, + shouldApplyTo: []*storage.Deployment{stackRoxNSDep, defaultNSDep, appStackRoxDep}, + }, + { + desc: "only stackrox ns, with an image-only exclusion", + scopes: []*storage.Scope{stackRoxNSScope}, + exclusions: []*storage.Exclusion{{Image: &storage.Exclusion_Image{Name: "docker.io/library/unrelated"}}}, + shouldApplyTo: []*storage.Deployment{stackRoxNSDep, appStackRoxDep}, + }, { desc: "only default ns", scopes: []*storage.Scope{defaultNSScope}, diff --git a/pkg/detection/deploytime/detector_impl_test.go b/pkg/detection/deploytime/detector_impl_test.go new file mode 100644 index 0000000000000..4a8d896e7c550 --- /dev/null +++ b/pkg/detection/deploytime/detector_impl_test.go @@ -0,0 +1,138 @@ +package deploytime + +import ( + "context" + "testing" + "time" + + "github.com/stackrox/rox/generated/storage" + "github.com/stackrox/rox/pkg/booleanpolicy" + "github.com/stackrox/rox/pkg/booleanpolicy/fieldnames" + "github.com/stackrox/rox/pkg/booleanpolicy/policyversion" + "github.com/stackrox/rox/pkg/detection" + "github.com/stackrox/rox/pkg/protocompat" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + excludedImage = "registry.example.com/team/legacy-app:1.0" + vulnerableImage = "registry.example.com/team/payments:2.3" +) + +// oldCriticalCVEPolicy mirrors a "Block Critical CVE older than 180 days" policy that runs at +// both the build and deploy stages and excludes a single image. +func oldCriticalCVEPolicy(exclusions ...*storage.Exclusion) *storage.Policy { + return &storage.Policy{ + Id: "old-critical-cve", + Name: "Block Critical CVE older than 180 days", + PolicyVersion: policyversion.CurrentVersion().String(), + Severity: storage.Severity_CRITICAL_SEVERITY, + LifecycleStages: []storage.LifecycleStage{storage.LifecycleStage_BUILD, storage.LifecycleStage_DEPLOY}, + Exclusions: exclusions, + PolicySections: []*storage.PolicySection{{ + PolicyGroups: []*storage.PolicyGroup{ + {FieldName: fieldnames.Severity, Values: []*storage.PolicyValue{{Value: "CRITICAL"}}}, + {FieldName: fieldnames.DaysSinceImageFirstDiscovered, Values: []*storage.PolicyValue{{Value: "180"}}}, + }, + }}, + } +} + +func imageExclusion(fullName string) *storage.Exclusion { + return &storage.Exclusion{Image: &storage.Exclusion_Image{Name: fullName}} +} + +func imageWithOldCriticalCVE(t *testing.T, fullName string) *storage.Image { + firstSeen, err := protocompat.ConvertTimeToTimestampOrError(time.Now().AddDate(0, 0, -365)) + require.NoError(t, err) + return &storage.Image{ + Id: fullName + "-sha", + Name: &storage.ImageName{FullName: fullName}, + Scan: &storage.ImageScan{ + Components: []*storage.EmbeddedImageScanComponent{{ + Name: "openssl", + Version: "1.0.1", + Vulns: []*storage.EmbeddedVulnerability{{ + Cve: "CVE-2014-0160", + Severity: storage.VulnerabilitySeverity_CRITICAL_VULNERABILITY_SEVERITY, + FirstImageOccurrence: firstSeen, + }}, + }}, + }, + } +} + +func deploymentRunning(name string, image *storage.Image) booleanpolicy.EnhancedDeployment { + return booleanpolicy.EnhancedDeployment{ + Deployment: &storage.Deployment{ + Id: name + "-id", + Name: name, + Namespace: "shop", + ClusterId: "cluster-1", + Containers: []*storage.Container{{ + Name: name, + Image: &storage.ContainerImage{Id: image.GetId(), Name: image.GetName()}, + }}, + }, + Images: []*storage.Image{image}, + } +} + +func detectorFor(t *testing.T, policy *storage.Policy) Detector { + policySet := detection.NewPolicySet(nil, nil) + require.NoError(t, policySet.UpsertPolicy(policy)) + return NewDetector(policySet) +} + +// TestImageExclusionDoesNotDisableDeployTimeDetection reproduces the customer report: adding an +// image exclusion to a build and deploy policy stopped the policy from alerting on every deployment, +// including deployments that do not run the excluded image. +func TestImageExclusionDoesNotDisableDeployTimeDetection(t *testing.T) { + ctx := context.Background() + payments := deploymentRunning("payments", imageWithOldCriticalCVE(t, vulnerableImage)) + + t.Run("without an exclusion, the vulnerable deployment alerts", func(t *testing.T) { + alerts, err := detectorFor(t, oldCriticalCVEPolicy()).Detect(ctx, payments) + require.NoError(t, err) + assert.Len(t, alerts, 1) + }) + + t.Run("with an image exclusion, a deployment that does not run the excluded image still alerts", func(t *testing.T) { + policy := oldCriticalCVEPolicy(imageExclusion(excludedImage)) + alerts, err := detectorFor(t, policy).Detect(ctx, payments) + require.NoError(t, err) + require.Len(t, alerts, 1) + assert.Equal(t, "payments", alerts[0].GetDeployment().GetName()) + }) + + t.Run("with an image exclusion, a deployment running the excluded image still alerts", func(t *testing.T) { + // Image exclusions only apply at build time, so a deployment running the excluded image is + // still evaluated at deploy time. + legacy := deploymentRunning("legacy-app", imageWithOldCriticalCVE(t, excludedImage)) + policy := oldCriticalCVEPolicy(imageExclusion(excludedImage)) + alerts, err := detectorFor(t, policy).Detect(ctx, legacy) + require.NoError(t, err) + require.Len(t, alerts, 1) + assert.Equal(t, "legacy-app", alerts[0].GetDeployment().GetName()) + }) + + t.Run("a deployment exclusion still skips the named deployment", func(t *testing.T) { + policy := oldCriticalCVEPolicy(&storage.Exclusion{Deployment: &storage.Exclusion_Deployment{Name: "payments"}}) + alerts, err := detectorFor(t, policy).Detect(ctx, payments) + require.NoError(t, err) + assert.Empty(t, alerts) + }) +} + +// TestImageExclusionStillAppliesAtBuildTime checks that the fix does not change build-time +// behavior: the excluded image is skipped, other images are still checked. +func TestImageExclusionStillAppliesAtBuildTime(t *testing.T) { + ctx := context.Background() + policy := oldCriticalCVEPolicy(imageExclusion(excludedImage)) + compiled, err := detection.CompilePolicy(policy, nil, nil) + require.NoError(t, err) + + assert.False(t, compiled.AppliesTo(ctx, imageWithOldCriticalCVE(t, excludedImage))) + assert.True(t, compiled.AppliesTo(ctx, imageWithOldCriticalCVE(t, vulnerableImage))) +} diff --git a/pkg/detection/exclusion_test.go b/pkg/detection/exclusion_test.go index f075901acf537..0844f5e17f527 100644 --- a/pkg/detection/exclusion_test.go +++ b/pkg/detection/exclusion_test.go @@ -125,6 +125,18 @@ func TestMatchesDeploymentExclusion(t *testing.T) { }, shouldMatch: false, }, + { + name: "Image-only exclusion does not match deployments", + deployment: fixtures.GetDeployment(), + policy: &storage.Policy{ + Exclusions: []*storage.Exclusion{ + { + Image: &storage.Exclusion_Image{Name: "docker.io/library/nginx"}, + }, + }, + }, + shouldMatch: false, + }, { name: "Scoped excluded scope, but different name", deployment: fixtures.GetDeployment(), @@ -248,3 +260,13 @@ func TestMatchesImageExclusion(t *testing.T) { }) } } + +func TestImageOnlyExclusionDoesNotMatchAuditEvent(t *testing.T) { + cx, err := newCompiledExclusion(&storage.Exclusion{Image: &storage.Exclusion_Image{Name: "docker.io/library/nginx"}}) + require.NoError(t, err) + + auditEvent := &storage.KubernetesEvent{ + Object: &storage.KubernetesEvent_Object{Name: "my-secret", Namespace: "default", ClusterId: "cluster-1"}, + } + assert.False(t, auditEventMatchesExclusions(context.Background(), auditEvent, []*compiledExclusion{cx})) +}