Skip to content

Integrate SecurityException CRDs into vulnerability scanning#342

Merged
slashben merged 5 commits intomainfrom
feature/security-exception-integration
Apr 16, 2026
Merged

Integrate SecurityException CRDs into vulnerability scanning#342
slashben merged 5 commits intomainfrom
feature/security-exception-integration

Conversation

@slashben
Copy link
Copy Markdown
Contributor

@slashben slashben commented Apr 12, 2026

Summary

  • Adds SecurityExceptionAdapter using dynamic client to read SecurityException and ClusterSecurityException CRDs
  • Converts CRD exceptions to armotypes.VulnerabilityExceptionPolicy for the existing matching flow
  • Merges CRD exceptions with cloud exceptions in GetCVEExceptions()
  • Handles expiry (expiresAt), expiredOnFix, and resource matching
  • Wired at startup via in-cluster config

Note: go.mod currently has a replace directive pointing to the local storage checkout. This needs to be updated to the tagged storage release after kubescape/storage#309 is merged.

Depends on:

Part of NAUT-1258: GitOps-native risk acceptance via SecurityException CRDs.

Test plan

  • Conversion tests: basic CVE, expiry skip, expiredOnFix flag, resource matching (6 tests passing)
  • Merge test: cloud + CRD exceptions combined
  • Remove replace directive after storage release is tagged
  • go build ./... passes (currently blocked by replace directive — will pass after storage tag)
  • E2E: apply a SecurityException CRD, trigger scan, verify CVE is excepted

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • Kubernetes SecurityException CRDs: Define vulnerability exception policies directly in your cluster
    • Unified exception management: Cloud and cluster-based exceptions are merged and applied together
    • Exception lifecycle: Configure expiration dates and automatic expiration when vulnerabilities are fixed

Implement adapter to fetch SecurityException and ClusterSecurityException
CRDs from the cluster and merge them into the existing CVE exception
pipeline. This enables GitOps-native vulnerability risk acceptance using
Kubernetes custom resources.

- Task 9: SecurityExceptionAdapter with dynamic client, JSON round-trip
  deserialization, expiry filtering, and CRD-to-armotypes conversion
- Task 10: BackendAdapter integration merging CRD exceptions alongside
  cloud-based exceptions in GetCVEExceptions()
- Task 11: Startup wiring in cmd/http/main.go with graceful fallback

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ben <ben@armosec.io>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 12, 2026

📝 Walkthrough

Walkthrough

This PR integrates Kubernetes SecurityException CRDs with the backend CVE exception handling. It introduces new CRD type definitions, a repository interface for reading exceptions, API server and no-op implementations, conversion logic to transform CRD exceptions into VulnerabilityExceptionPolicy format, and merges them into GetCVEExceptions alongside cloud-based exceptions with graceful error handling.

Changes

Cohort / File(s) Summary
Kubernetes CRD Type Definitions
pkg/securityexception/v1beta1/types.go
Defines SecurityException and ClusterSecurityException CRD types with specs for vulnerabilities, posture exceptions, workload selection, and expiration handling; includes enum types for VulnerabilityStatus and PostureAction.
Repository Port & Implementations
core/ports/repositories.go, repositories/apiserver.go, repositories/noop_security_exception.go
Adds SecurityExceptionRepository interface for reading exceptions; implements it via API server dynamic client (with protobuf and JSON marshaling) and a no-op variant for CRD-unavailable environments.
Conversion & Helper Logic
adapters/v1/securityexception.go, adapters/v1/securityexception_test.go
Implements conversion from CRD SecurityException/ClusterSecurityException to VulnerabilityExceptionPolicy format; handles expiration filtering, designator building, and expiredOnFix propagation with comprehensive test coverage.
Backend Adapter Integration
adapters/v1/backend.go, adapters/v1/backend_test.go
Adds securityExceptionRepo dependency to BackendAdapter; extends GetCVEExceptions to fetch and merge CRD exceptions with cloud exceptions; logs warnings on fetch failures and preserves merge order.
Wiring & Configuration
cmd/http/main.go, .tool-versions
Wires up SecurityExceptionRepository based on storage availability (API server when available, no-op fallback); adds Go 1.25.6 toolchain version specification.

Sequence Diagram

sequenceDiagram
    participant Client
    participant BackendAdapter
    participant getCVEExceptionsFunc as getCVEExceptionsFunc
    participant SecurityExceptionRepo
    participant K8sAPI as Kubernetes API
    participant Converter

    Client->>BackendAdapter: GetCVEExceptions(ctx)
    activate BackendAdapter
    BackendAdapter->>getCVEExceptionsFunc: Call cloud exception fetcher
    activate getCVEExceptionsFunc
    getCVEExceptionsFunc-->>BackendAdapter: vulnExceptionList (cloud-based)
    deactivate getCVEExceptionsFunc
    
    BackendAdapter->>BackendAdapter: Extract namespace from context
    BackendAdapter->>SecurityExceptionRepo: GetSecurityExceptions(ctx, namespace)
    activate SecurityExceptionRepo
    SecurityExceptionRepo->>K8sAPI: List SecurityException CRDs
    SecurityExceptionRepo->>K8sAPI: List ClusterSecurityException CRDs
    K8sAPI-->>SecurityExceptionRepo: Exceptions data
    SecurityExceptionRepo-->>BackendAdapter: (seList, cseList, error)
    deactivate SecurityExceptionRepo
    
    alt Exceptions fetched successfully & non-empty
        BackendAdapter->>Converter: convertToVulnerabilityExceptionPolicies(seList, cseList)
        activate Converter
        Converter->>Converter: Filter expired, build policies per vulnerability
        Converter->>Converter: Build designators with namespace/resource info
        Converter-->>BackendAdapter: crdPolicies
        deactivate Converter
        BackendAdapter->>BackendAdapter: Append crdPolicies to vulnExceptionList
    else Fetch fails
        BackendAdapter->>BackendAdapter: Log warning, proceed without CRD policies
    end
    
    BackendAdapter-->>Client: Merged vulnExceptionList (cloud + CRD)
    deactivate BackendAdapter
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 Whiskers twitching with delight,
CRD exceptions now in sight!
Cloud and Kubernetes combined,
Merged with care, no vuln left behind,
Hopping forward, code takes flight! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main objective: integrating SecurityException CRDs into vulnerability scanning, which is the primary focus of all changes across multiple files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/security-exception-integration

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
adapters/v1/backend.go (1)

67-70: Guard SetSecurityExceptionAdapter against nil input.

If a nil adapter is passed, Line 69 stores a method value on a nil receiver and can panic later when invoked.

Proposed defensive fix
 func (a *BackendAdapter) SetSecurityExceptionAdapter(adapter *SecurityExceptionAdapter) {
-	a.securityExceptionAdapter = adapter
-	a.getSecurityExceptionsFunc = adapter.GetSecurityExceptions
+	a.securityExceptionAdapter = adapter
+	if adapter == nil {
+		a.getSecurityExceptionsFunc = nil
+		return
+	}
+	a.getSecurityExceptionsFunc = adapter.GetSecurityExceptions
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@adapters/v1/backend.go` around lines 67 - 70, Guard
SetSecurityExceptionAdapter against nil by checking the adapter parameter before
assigning fields: in BackendAdapter.SetSecurityExceptionAdapter, if adapter is
nil, set a.securityExceptionAdapter and a.getSecurityExceptionsFunc to nil (or
leave them unchanged if that’s desired) and return; otherwise assign
a.securityExceptionAdapter = adapter and a.getSecurityExceptionsFunc =
adapter.GetSecurityExceptions. This prevents storing a method value bound to a
nil receiver and avoids panics when invoking getSecurityExceptionsFunc.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@adapters/v1/securityexception.go`:
- Around line 156-170: The loop building PortalDesignator entries can append a
designator with no attributes (attrs map empty), causing overly broad matches;
update the code in the for _, r := range resources loop that creates
identifiers.PortalDesignator (DesignatorType: identifiers.DesignatorAttributes,
Attributes: attrs) to only append the designator when attrs contains at least
one key (e.g., check len(attrs) > 0) so empty-attribute designators are not
emitted.
- Around line 42-45: In GetSecurityExceptions, guard against an empty namespace
before calling
a.client.Resource(securityExceptionGVR).Namespace(namespace).List: if namespace
is empty, do not call Namespace(...) (and instead return empty
[]sev1.SecurityException and []sev1.ClusterSecurityException results
immediately), otherwise proceed to call Namespace(namespace).List as before to
ensure you only list namespaced SecurityExceptions when a namespace is
explicitly provided.

In `@go.mod`:
- Line 406: Update the otel SDK dependency to a patched release by changing the
module version for go.opentelemetry.io/otel/sdk from v1.42.0 to v1.43.0 (or
newer) in go.mod, then run the Go module tooling (e.g., go get
go.opentelemetry.io/otel/sdk@v1.43.0 and go mod tidy) to update go.sum and
vendor state; reference the dependency name go.opentelemetry.io/otel/sdk when
making the change.
- Line 458: The go.mod contains a local replace directive "replace
github.com/kubescape/storage => ../storage" which must be removed before merge;
delete that replace line and ensure the module resolves to a proper versioned
import (or add an explicit version requirement for github.com/kubescape/storage
if needed) so builds are reproducible in CI and for consumers.

---

Nitpick comments:
In `@adapters/v1/backend.go`:
- Around line 67-70: Guard SetSecurityExceptionAdapter against nil by checking
the adapter parameter before assigning fields: in
BackendAdapter.SetSecurityExceptionAdapter, if adapter is nil, set
a.securityExceptionAdapter and a.getSecurityExceptionsFunc to nil (or leave them
unchanged if that’s desired) and return; otherwise assign
a.securityExceptionAdapter = adapter and a.getSecurityExceptionsFunc =
adapter.GetSecurityExceptions. This prevents storing a method value bound to a
nil receiver and avoids panics when invoking getSecurityExceptionsFunc.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ccf3fc88-1bce-47a1-bf4d-27a7bb411fc7

📥 Commits

Reviewing files that changed from the base of the PR and between 09085cc and c44bd45.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (6)
  • adapters/v1/backend.go
  • adapters/v1/backend_test.go
  • adapters/v1/securityexception.go
  • adapters/v1/securityexception_test.go
  • cmd/http/main.go
  • go.mod

Comment thread adapters/v1/securityexception.go Outdated
Comment thread adapters/v1/securityexception.go
Comment thread go.mod
go.opentelemetry.io/otel/exporters/stdout/stdouttrace v1.42.0 // indirect
go.opentelemetry.io/otel/log v0.18.0 // indirect
go.opentelemetry.io/otel/metric v1.42.0 // indirect
go.opentelemetry.io/otel/sdk v1.42.0 // indirect
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

What is the first patched version of go.opentelemetry.io/otel/sdk for GHSA-hfvc-g4fc-pqhx, and is v1.42.0 affected?

💡 Result:

The first patched version of go.opentelemetry.io/otel/sdk for GHSA-hfvc-g4fc-pqhx is v1.43.0. Yes, v1.42.0 is affected.

Citations:


Bump go.opentelemetry.io/otel/sdk from v1.42.0 to v1.43.0 or later to address GHSA-hfvc-g4fc-pqhx.

The version currently used (v1.42.0) is affected by GHSA-hfvc-g4fc-pqhx. The first patched version is v1.43.0.

🧰 Tools
🪛 OSV Scanner (2.3.5)

[HIGH] 406-406: go.opentelemetry.io/otel/sdk 1.42.0: opentelemetry-go: BSD kenv command not using absolute path enables PATH hijacking

(GHSA-hfvc-g4fc-pqhx)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@go.mod` at line 406, Update the otel SDK dependency to a patched release by
changing the module version for go.opentelemetry.io/otel/sdk from v1.42.0 to
v1.43.0 (or newer) in go.mod, then run the Go module tooling (e.g., go get
go.opentelemetry.io/otel/sdk@v1.43.0 and go mod tidy) to update go.sum and
vendor state; reference the dependency name go.opentelemetry.io/otel/sdk when
making the change.

Comment thread go.mod Outdated
Define CRD types locally in pkg/securityexception/v1/ instead of
importing from kubescape/storage. This avoids the aggregated API vs
CRD confusion — types are plain Go structs with JSON tags, no scheme
registration, no deepcopy generation, no codegen.

- Add pkg/securityexception/v1/types.go with all CRD types
- Update imports from storage to local package
- Remove go.mod replace directive for storage

Closes dependency on kubescape/storage#309 (now closed).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ben <ben@armosec.io>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
go.mod (1)

406-406: ⚠️ Potential issue | 🟠 Major

Bump vulnerable go.opentelemetry.io/otel/sdk to a patched version.

Line 406 still pins go.opentelemetry.io/otel/sdk to v1.42.0, which is flagged as affected by GHSA-hfvc-g4fc-pqhx. Please move to v1.43.0 or later.

Proposed fix
-	go.opentelemetry.io/otel/sdk v1.42.0 // indirect
+	go.opentelemetry.io/otel/sdk v1.43.0 // indirect
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@go.mod` at line 406, Update the vulnerable dependency declaration for
go.opentelemetry.io/otel/sdk in go.mod from v1.42.0 to at least v1.43.0 (or a
newer patched release); edit the version string for the module entry
"go.opentelemetry.io/otel/sdk" and then run module tooling (e.g., go get
go.opentelemetry.io/otel/sdk@v1.43.0 and go mod tidy) to ensure the lockfile and
transitive deps are updated.
🧹 Nitpick comments (1)
pkg/securityexception/v1/types.go (1)

7-101: Keep this CRD contract single-sourced.

This file is now a hand-maintained copy of a public schema. If the chart/storage definition changes first, unmarshalling will still succeed and the adapter can silently drift. Once the shared package is tagged again, prefer importing the canonical types or add a parity test against the shipped CRD schema.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@pkg/securityexception/v1/types.go` around lines 7 - 101, This CRD type file
is a hand-maintained duplicate of the canonical schema and can drift; replace
the duplicated definitions (e.g., SecurityException, ClusterSecurityException,
SecurityExceptionSpec, ExceptionMatch, ResourceMatch, VulnerabilityException,
VulnerabilityRef, PostureException, VulnerabilityStatus, PostureAction) by
importing the single-source canonical types from the shared CRD package once
that package is tagged (update imports and go.mod accordingly) or, if you cannot
import yet, add an automated parity test that compares these local structs/JSON
tags against the shipped CRD schema to fail CI on drift; remove or mark the
duplicated definitions as temporary and document the intent to switch to the
shared package when it’s available.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@adapters/v1/securityexception.go`:
- Around line 42-71: The GetSecurityExceptions method currently fails hard when
the CRD-backed List calls error; change the logic so that failures to list
securityExceptionGVR or clusterSecurityExceptionGVR do not return an overall
error but instead treat the missing source as an empty result. Specifically, in
GetSecurityExceptions handle errors from
a.client.Resource(...).Namespace(namespace).List(...) and
a.client.Resource(clusterSecurityExceptionGVR).List(...) by setting the
corresponding sev1.SecurityExceptionList / sev1.ClusterSecurityExceptionList to
empty and continuing (only return an error for unexpected/non-CRD-missing cases
if you distinguish them), ensuring the function returns empty slices for those
types rather than failing completely.

---

Duplicate comments:
In `@go.mod`:
- Line 406: Update the vulnerable dependency declaration for
go.opentelemetry.io/otel/sdk in go.mod from v1.42.0 to at least v1.43.0 (or a
newer patched release); edit the version string for the module entry
"go.opentelemetry.io/otel/sdk" and then run module tooling (e.g., go get
go.opentelemetry.io/otel/sdk@v1.43.0 and go mod tidy) to ensure the lockfile and
transitive deps are updated.

---

Nitpick comments:
In `@pkg/securityexception/v1/types.go`:
- Around line 7-101: This CRD type file is a hand-maintained duplicate of the
canonical schema and can drift; replace the duplicated definitions (e.g.,
SecurityException, ClusterSecurityException, SecurityExceptionSpec,
ExceptionMatch, ResourceMatch, VulnerabilityException, VulnerabilityRef,
PostureException, VulnerabilityStatus, PostureAction) by importing the
single-source canonical types from the shared CRD package once that package is
tagged (update imports and go.mod accordingly) or, if you cannot import yet, add
an automated parity test that compares these local structs/JSON tags against the
shipped CRD schema to fail CI on drift; remove or mark the duplicated
definitions as temporary and document the intent to switch to the shared package
when it’s available.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: aa2a0b7d-e4cd-4391-bd4d-617b3377b5a9

📥 Commits

Reviewing files that changed from the base of the PR and between c44bd45 and 2277976.

⛔ Files ignored due to path filters (1)
  • go.sum is excluded by !**/*.sum
📒 Files selected for processing (7)
  • .tool-versions
  • adapters/v1/backend.go
  • adapters/v1/backend_test.go
  • adapters/v1/securityexception.go
  • adapters/v1/securityexception_test.go
  • go.mod
  • pkg/securityexception/v1/types.go
✅ Files skipped from review due to trivial changes (1)
  • .tool-versions
🚧 Files skipped from review as they are similar to previous changes (1)
  • adapters/v1/backend.go

Comment thread adapters/v1/securityexception.go Outdated
Comment on lines +42 to +71
func (a *SecurityExceptionAdapter) GetSecurityExceptions(ctx context.Context, namespace string) ([]sev1.SecurityException, []sev1.ClusterSecurityException, error) {
// List namespaced SecurityExceptions
seUnstructured, err := a.client.Resource(securityExceptionGVR).Namespace(namespace).List(ctx, metav1.ListOptions{})
if err != nil {
return nil, nil, err
}

raw, err := json.Marshal(seUnstructured)
if err != nil {
return nil, nil, err
}
var seList sev1.SecurityExceptionList
if err := json.Unmarshal(raw, &seList); err != nil {
return nil, nil, err
}

// List cluster-scoped ClusterSecurityExceptions
cseUnstructured, err := a.client.Resource(clusterSecurityExceptionGVR).List(ctx, metav1.ListOptions{})
if err != nil {
return nil, nil, err
}

raw, err = json.Marshal(cseUnstructured)
if err != nil {
return nil, nil, err
}
var cseList sev1.ClusterSecurityExceptionList
if err := json.Unmarshal(raw, &cseList); err != nil {
return nil, nil, err
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not make the CRD source fatal when it is unavailable.

Both List calls fail hard here. Because the CRDs/RBAC roll out separately, a cluster can run this binary before that source exists; in that state, CRD-backed exceptions should degrade to empty lists instead of breaking exception retrieval entirely.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@adapters/v1/securityexception.go` around lines 42 - 71, The
GetSecurityExceptions method currently fails hard when the CRD-backed List calls
error; change the logic so that failures to list securityExceptionGVR or
clusterSecurityExceptionGVR do not return an overall error but instead treat the
missing source as an empty result. Specifically, in GetSecurityExceptions handle
errors from a.client.Resource(...).Namespace(namespace).List(...) and
a.client.Resource(clusterSecurityExceptionGVR).List(...) by setting the
corresponding sev1.SecurityExceptionList / sev1.ClusterSecurityExceptionList to
empty and continuing (only return an error for unexpected/non-CRD-missing cases
if you distinguish them), ensuring the function returns empty slices for those
types rather than failing completely.

Comment thread adapters/v1/securityexception.go Outdated
Comment thread adapters/v1/backend.go Outdated

// SetSecurityExceptionAdapter configures the adapter to merge CRD-based
// SecurityException resources into the vulnerability exception pipeline.
func (a *BackendAdapter) SetSecurityExceptionAdapter(adapter *SecurityExceptionAdapter) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetSecurityExceptionAdapter is called once at startup and creates a nil-able dependency that needs a nil check on every call. This is unusual Go — either pass the adapter through NewBackendAdapter as an optional parameter (variadic or functional option), or create a no-op adapter that satisfies the interface. The nil guard pattern spreads through callers.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 999792f — removed setter entirely. Repository is passed via constructor. Added NoOpSecurityExceptionRepository for test/local mode.

Comment thread adapters/v1/backend.go Outdated
sendStatusFunc func(*backendClientV1.BaseReportSender, string, bool)
accessKey string
securityExceptionAdapter *SecurityExceptionAdapter
getSecurityExceptionsFunc func(ctx context.Context, namespace string) ([]sev1.SecurityException, []sev1.ClusterSecurityException, error)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BackendAdapter stores both securityExceptionAdapter *SecurityExceptionAdapter and getSecurityExceptionsFunc func(...). The func is always set to adapter.GetSecurityExceptions. Since the adapter is stored too, the func field adds no flexibility while doubling the nil-check surface.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 999792f — replaced both fields with a single securityExceptionRepo ports.SecurityExceptionRepository interface.

Comment thread adapters/v1/securityexception.go Outdated
return nil, nil, err
}

raw, err = json.Marshal(cseUnstructured)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k8s.io/apimachinery/pkg/runtime provides DefaultUnstructuredConverter.FromUnstructured which avoids the intermediate JSON round-trip.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 999792f — using runtime.DefaultUnstructuredConverter.FromUnstructured now, no more JSON round-trip.

Comment thread adapters/v1/securityexception.go Outdated

// SecurityExceptionAdapter fetches SecurityException and ClusterSecurityException
// CRDs from the cluster via the dynamic client.
type SecurityExceptionAdapter struct {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would implement this inside repositories/apiserver.go (and the mock interfaces) as we already have the client created there.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 999792f — moved CRD fetching to repositories/apiserver.go. Added DynamicClient field to APIServerStore, created alongside the existing typed client in NewAPIServerStorage.

slashben and others added 2 commits April 15, 2026 13:14
…ection

- Move CRD fetching from adapter to repositories/apiserver.go
- Use runtime.DefaultUnstructuredConverter instead of JSON round-trip
- Rename types from v1 to v1beta1 (new API, may evolve)
- Replace setter+nil-check with constructor injection
- Add NoOpSecurityExceptionRepository for test/local mode
- Guard empty namespace to prevent cross-namespace listing
- Graceful degradation: CRD list errors log warning, return empty
- Skip empty-attribute designators

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ben <ben@armosec.io>
Resolve go.mod/go.sum conflicts by accepting main's dependency
versions and running go mod tidy.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ben <ben@armosec.io>
Copy link
Copy Markdown
Contributor

@matthyx matthyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

almost there

Comment thread repositories/apiserver.go
Comment thread repositories/noop_security_exception.go
@slashben slashben requested a review from matthyx April 15, 2026 17:22
@github-actions
Copy link
Copy Markdown

Summary:

  • License scan: failure
  • Credentials scan: failure
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: failure

- Add fakedynamic.NewSimpleDynamicClient to NewFakeAPIServerStorage
- Move NoOpSecurityExceptionRepository from adapters/v1 to repositories

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Ben <ben@armosec.io>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
adapters/v1/securityexception.go (1)

20-40: ⚠️ Potential issue | 🟠 Major

Fail closed on unsupported match fields to avoid scope widening.

Line 27/Line 38 eventually rely on Line 75–Line 112, which only maps resources (namespace/kind/name). namespaceSelector, objectSelector, images, and resource.apiGroup are dropped, so a scoped CRD can become broader than intended.

Proposed fix (skip unsafe conversions until full mapping is supported)
 func convertToVulnerabilityExceptionPolicies(exceptions []sev1beta1.SecurityException, clusterExceptions []sev1beta1.ClusterSecurityException) []armotypes.VulnerabilityExceptionPolicy {
 	var policies []armotypes.VulnerabilityExceptionPolicy

 	now := time.Now()

 	for i := range exceptions {
 		se := &exceptions[i]
 		if isExpired(se.Spec.ExpiresAt, now) {
 			continue
 		}
+		if hasUnsupportedMatch(se.Spec.Match) {
+			continue
+		}
 		namespace := se.Namespace
 		for _, vuln := range se.Spec.Vulnerabilities {
 			p := buildPolicy(se.Spec, vuln, namespace)
 			policies = append(policies, p)
 		}
 	}

 	for i := range clusterExceptions {
 		cse := &clusterExceptions[i]
 		if isExpired(cse.Spec.ExpiresAt, now) {
 			continue
 		}
+		if hasUnsupportedMatch(cse.Spec.Match) {
+			continue
+		}
 		for _, vuln := range cse.Spec.Vulnerabilities {
 			p := buildPolicy(cse.Spec, vuln, "")
 			policies = append(policies, p)
 		}
 	}

 	return policies
 }
+
+func hasUnsupportedMatch(match sev1beta1.ExceptionMatch) bool {
+	if match.NamespaceSelector != nil || match.ObjectSelector != nil || len(match.Images) > 0 {
+		return true
+	}
+	for _, r := range match.Resources {
+		if r.APIGroup != "" {
+			return true
+		}
+	}
+	return false
+}

Also applies to: 70-112

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@adapters/v1/securityexception.go` around lines 20 - 40, The current loop that
converts exceptions into policies calls buildPolicy for every non-expired
SecurityException (exceptions/clusterExceptions) but buildPolicy only maps
resources (namespace/kind/name) and ignores namespaceSelector, objectSelector,
images, and resource.apiGroup, which can widen scope; update the conversion
loops that use isExpired and buildPolicy to first detect and skip any exception
whose Spec contains unsupported match fields (namespaceSelector, objectSelector,
images, or resource.apiGroup) so we "fail closed" — i.e., do not convert those
entries into policies until buildPolicy (or the mapping code around lines
75–112) is extended to handle those fields; ensure both the exceptions and
clusterExceptions loops perform the same safety check before calling
buildPolicy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@adapters/v1/securityexception.go`:
- Around line 20-40: The current loop that converts exceptions into policies
calls buildPolicy for every non-expired SecurityException
(exceptions/clusterExceptions) but buildPolicy only maps resources
(namespace/kind/name) and ignores namespaceSelector, objectSelector, images, and
resource.apiGroup, which can widen scope; update the conversion loops that use
isExpired and buildPolicy to first detect and skip any exception whose Spec
contains unsupported match fields (namespaceSelector, objectSelector, images, or
resource.apiGroup) so we "fail closed" — i.e., do not convert those entries into
policies until buildPolicy (or the mapping code around lines 75–112) is extended
to handle those fields; ensure both the exceptions and clusterExceptions loops
perform the same safety check before calling buildPolicy.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6eea3d40-05d0-4b4c-8b25-51185f418893

📥 Commits

Reviewing files that changed from the base of the PR and between 2277976 and e4cc551.

📒 Files selected for processing (9)
  • adapters/v1/backend.go
  • adapters/v1/backend_test.go
  • adapters/v1/securityexception.go
  • adapters/v1/securityexception_test.go
  • cmd/http/main.go
  • core/ports/repositories.go
  • pkg/securityexception/v1beta1/types.go
  • repositories/apiserver.go
  • repositories/noop_security_exception.go
✅ Files skipped from review due to trivial changes (1)
  • adapters/v1/securityexception_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
  • adapters/v1/backend_test.go
  • adapters/v1/backend.go

@github-actions
Copy link
Copy Markdown

Summary:

  • License scan: failure
  • Credentials scan: failure
  • Vulnerabilities scan: failure
  • Unit test: success
  • Go linting: failure

@matthyx matthyx moved this from WIP to Needs Reviewer in KS PRs tracking Apr 16, 2026
@slashben slashben merged commit 7d58f1b into main Apr 16, 2026
11 checks passed
@slashben slashben deleted the feature/security-exception-integration branch April 16, 2026 12:57
@matthyx matthyx moved this from Needs Reviewer to To Archive in KS PRs tracking Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants