feat: replace legacy exceptions with SecurityException CRDs#86
Draft
mgalesloot wants to merge 1 commit into
Draft
feat: replace legacy exceptions with SecurityException CRDs#86mgalesloot wants to merge 1 commit into
mgalesloot wants to merge 1 commit into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughMigrates exception handling from ConfigMap-based groups to SecurityException/ClusterSecurityException CRDs, adds CRD models and client-side application of exceptions, introduces guided exception creation UIs, removes legacy exception-group management, and updates compliance/vulnerability views and routing. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Exception Form Dialog
participant App as Headlamp Plugin
participant K8s as Kubernetes API
participant Snackbar as Notification
User->>UI: Fill metadata & select scope/action
UI->>App: Validate & construct CRD payload
App->>K8s: POST SecurityException/ClusterSecurityException
alt API Success
K8s-->>App: Created
App->>Snackbar: show success
App->>UI: close dialog
else API Failure
K8s-->>App: Error
App->>Snackbar: show error
end
sequenceDiagram
participant Compliance as Compliance Page
participant Fetcher as fetchSecurityExceptions()
participant K8s as Kubernetes API
participant Applier as applySecurityExceptionsToWorkloadScans
participant State as Workload Scan State
Compliance->>Fetcher: request exceptions + namespace labels
Fetcher->>K8s: list namespaced SecurityException
Fetcher->>K8s: list ClusterSecurityException
Fetcher->>K8s: list Namespaces
K8s-->>Fetcher: exceptions + namespaces
Fetcher-->>Compliance: exceptions & labels
Compliance->>Applier: apply exceptions to scans
Applier->>State: update exceptedByPolicy flags on workloads/controls
State-->>Compliance: re-render updated views
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/compliance/Compliance.tsx (1)
117-139:⚠️ Potential issue | 🟡 Minor
fetchData()is fire-and-forget — unhandled rejection if any step throws.
fetchData()on Line 135 is invoked without.catch(...)(and the cleanup function returned alongside also can't await it). Most failure paths inside it are guarded (fetchSecurityExceptions().catch(...), therequestcall insidefetchCustomFrameworkshas.catch), but two remain unguarded:
applySecurityExceptionsToWorkloadScans(...)is synchronous and would reject the surrounding async function on any data-shape surprise.- Inside
fetchCustomFrameworks,JSON.parse(configMap.data.controlsIDs)(Line 437) will throw on a malformed/missing field, also rejectingfetchData.When
fetchDatarejects there's no UI feedback andsetLoadingmay staytrueforever. Suggest attaching a.catch(and ideally surfacing it via an error state, similar toVulnerabilities.tsx).🛡️ Suggested guard
- fetchData(); + fetchData().catch(error => { + console.error('Failed to load compliance data', error); + setLoading(false); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/compliance/Compliance.tsx` around lines 117 - 139, fetchData is being called fire-and-forget so any thrown error (from applySecurityExceptionsToWorkloadScans or JSON.parse inside fetchCustomFrameworks) will leave setLoading stuck and provide no UI feedback; wrap the fetchData invocation with a proper promise chain (e.g., call fetchData().catch(err => { setError(err); }).finally(() => setLoading(false))) or otherwise handle errors returned by fetchData, add an error state (e.g., setError) to surface the message similar to Vulnerabilities.tsx, and ensure setLoading is cleared in a finally block; specifically reference fetchData, applySecurityExceptionsToWorkloadScans, fetchCustomFrameworks, fetchSecurityExceptions and setLoading when making these changes so the async error paths are caught and UI state is updated.
🧹 Nitpick comments (14)
src/vulnerabilities/WorkloadScanDetails.tsx (1)
97-105: Match repo convention: wrapMatchesprops inReadonly<>.The sibling components in this PR (
WorkloadsinCVEResults.tsx,Controlsincompliance/WorkloadScanDetails.tsx, etc.) all useReadonly<{ ... }>for their props. AddingReadonly<>here keeps the convention consistent and reaffirms the "props are immutable" expectation.♻️ Suggested change
-function Matches(props: { +function Matches(props: Readonly<{ manifest: VulnerabilityManifest; relevant: VulnerabilityManifest | null; workloadName: string; workloadNamespace: string; workloadKind: string; imageRef?: string; -}) { +}>) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vulnerabilities/WorkloadScanDetails.tsx` around lines 97 - 105, Wrap the props type for the Matches component in Readonly<> to match repository convention (i.e., change the parameter type from the inline object to Readonly<{ ... }>) so the props are treated as immutable; keep the property names and types the same (manifest, relevant, workloadName, workloadNamespace, workloadKind, imageRef) and leave the existing destructuring in the Matches function body unchanged.src/compliance/ResourceList.tsx (1)
10-20: Drop the now-unusedsetWorkloadScanDataprop from the type.Same situation as in
NamespaceView.tsx: after removing the toggle-driven mutation flow, this prop is never destructured or invoked. It's still being passed in fromCompliance.tsx(Line ~219). Remove it from both sides for a cleaner surface.♻️ Suggested cleanup
export default function KubescapeWorkloadConfigurationScanList( props: Readonly<{ workloadScanData: WorkloadConfigurationScanSummary[] | null; - setWorkloadScanData?: (workloadScanData: WorkloadConfigurationScanSummary[]) => void; framework: FrameWork; isFailedControlSwitchChecked: boolean; }> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/compliance/ResourceList.tsx` around lines 10 - 20, The prop type for KubescapeWorkloadConfigurationScanList still includes the unused setWorkloadScanData; remove setWorkloadScanData from the component props type declaration and from any places that pass it in (e.g., the caller in Compliance.tsx around where KubescapeWorkloadConfigurationScanList is instantiated) so the prop surface matches actual usage; update the Readonly<{ ... }> definition inside KubescapeWorkloadConfigurationScanList (and remove any passing of setWorkloadScanData in Compliance.tsx) to eliminate the unused prop.src/vulnerabilities/Vulnerabilities.tsx (1)
4-43: Fix import sort flagged by static analysis.The build pipeline is warning on imports here. The new
GuidedClusterVulnerabilityExceptionFormimport on Line 24 is wedged between the@mui/materialblock and thereactimport, which breaks the third-party-then-relative grouping used elsewhere in this file. Run the project's autofix (or move the import down to the relative-imports section).♻️ Suggested grouping
import { Box, Button, FormControlLabel, IconButton, Stack, Switch, Tooltip, Typography, } from '@mui/material'; -import { GuidedClusterVulnerabilityExceptionForm } from '../exceptions/GuidedClusterVulnerabilityExceptionForm'; import { useEffect, useRef, useState } from 'react'; import { isAllowedNamespaceUpdated } from '../common/clusterContext'; import { KubescapeConfig, kubescapeConfigStore } from '../common/config-store'; import { ErrorMessage } from '../common/ErrorMessage'; +import { GuidedClusterVulnerabilityExceptionForm } from '../exceptions/GuidedClusterVulnerabilityExceptionForm'; import { ProgressIndicator } from '../common/ProgressIndicator';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vulnerabilities/Vulnerabilities.tsx` around lines 4 - 43, The import ordering breaks the third-party-then-relative grouping by placing GuidedClusterVulnerabilityExceptionForm between `@mui/material` and React imports; move the GuidedClusterVulnerabilityExceptionForm import so all third-party imports (e.g., '@mui/material', 'react', '@iconify/react', etc.) remain grouped together and place GuidedClusterVulnerabilityExceptionForm with the other project-relative imports (near VulnerabilityManifest, ImageListView, WorkloadScanListView) or run the project's import-sort/autofix to restore the expected grouping.src/compliance/NamespaceView.tsx (1)
33-40: Drop the now-unusedsetWorkloadScanDataprop from the type.After this PR
setWorkloadScanDatais no longer destructured (Line 40) or used anywhere in this component — its only purpose was the removed namespace-exclusion mutation. Keeping it in the prop interface (even as optional) leaves a dead surface for callers, andCompliance.tsxis still passing it on Line 230. Cleaner to remove it from both sides.♻️ Suggested cleanup
export default function NamespaceView( props: Readonly<{ workloadScanData: WorkloadConfigurationScanSummary[] | null; - setWorkloadScanData?: (workloadScanData: WorkloadConfigurationScanSummary[]) => void; }> )And in
src/compliance/Compliance.tsx(Line ~228-231), drop the now-unneededsetWorkloadScanData={setWorkloadScanData}.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/compliance/NamespaceView.tsx` around lines 33 - 40, Remove the now-unused optional prop setWorkloadScanData from the NamespaceView component's props type and update any callers to stop passing it (e.g., remove setWorkloadScanData={setWorkloadScanData} in Compliance.tsx). Specifically, edit the NamespaceView declaration to only accept workloadScanData: WorkloadConfigurationScanSummary[] | null and remove setWorkloadScanData from the Readonly prop shape, then remove the corresponding prop usage in Compliance.tsx where it is passed through.src/compliance/Compliance.tsx (1)
12-24: Import grouping is broken; same class of issue flagged by static analysis onVulnerabilities.tsx.
@iconify/react(Line 12) lands in the middle of the@mui/materialblock, andGuidedClusterComplianceExceptionForm(Line 24) appears before sibling relative imports under../common/.... Run the project's import-sort autofix to keep this file consistent with the rest of the codebase.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/compliance/Compliance.tsx` around lines 12 - 24, Imports in Compliance.tsx are misordered: the named import Icon from '@iconify/react' is interleaved with the `@mui/material` imports and GuidedClusterComplianceExceptionForm is placed before sibling relative imports; run the project's import-sort autofix (or re-run the configured linter/import-sort) to reorder imports so external packages (e.g., '@iconify/react') group together, then `@mui/material` block follows, and local relative imports (including GuidedClusterComplianceExceptionForm and other ../common/... imports) appear last; ensure the symbols Icon and GuidedClusterComplianceExceptionForm remain unchanged while fixing import order.src/softwarecomposition/SecurityException.ts (2)
9-14: TightenmatchExpressionstyping.
matchExpressions?: any[]loses type safety; the underlying selector follows KubernetesLabelSelectorRequirement({ key: string; operator: 'In' | 'NotIn' | 'Exists' | 'DoesNotExist'; values?: string[] }). Defining a small interface here would let the form components and detail view rely on it instead ofany.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/softwarecomposition/SecurityException.ts` around lines 9 - 14, The matchExpressions fields in SecurityExceptionMatchSpec currently use any[] which loses type safety; define a small interface (e.g., LabelSelectorRequirement with properties key: string; operator: 'In' | 'NotIn' | 'Exists' | 'DoesNotExist'; values?: string[]) and replace matchExpressions?: any[] with matchExpressions?: LabelSelectorRequirement[]; update the two selector shapes (namespaceSelector and objectSelector) to reference this typed matchExpressions so form components and detail views can rely on the concrete type.
53-74: API group deviates from the documented Kubescape data-access guideline — please update CLAUDE.md.
SecurityExceptionandClusterSecurityExceptionare pinned toapiVersion: 'kubescape.io/v1beta1', which is correct for the operator's CRD. However, the repository's coding guideline (CLAUDE.md) states that.tsfiles should access data throughspdx.softwarecomposition.kubescape.io/v1beta1. Either widen that guideline to mention the newkubescape.io/v1beta1exception CRDs explicitly, or the next AI/code review cycle will keep flagging this.As per coding guidelines: "Applies to **/*.ts : Access all data through Kubescape CRDs in the
spdx.softwarecomposition.kubescape.io/v1beta1API group".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/softwarecomposition/SecurityException.ts` around lines 53 - 74, The CLAUDE.md guideline currently mandates using spdx.softwarecomposition.kubescape.io/v1beta1 for all .ts files but the code defines CRDs SecurityException and ClusterSecurityException with apiVersion 'kubescape.io/v1beta1'; update CLAUDE.md to explicitly allow kubescape.io/v1beta1 for the SecurityException and ClusterSecurityException CRDs (or broaden the rule to accept authorized operator CRDs beyond spdx.*) so future reviews don't flag SecurityException and ClusterSecurityException; mention the two symbols by name and the allowed apiVersion so the guideline matches the implementation.src/exceptions/GuidedComplianceExceptionForm.tsx (2)
67-114: Moveconst isClusterabovehandleSubmit— currently used before declaration.
handleSubmit(line 67) readsisClusterat line 89, but the binding is declared at line 114. This works only becausehandleSubmitis invoked asynchronously (on user click, after the component body has finished executing), so the TDZ has lifted by then. It's brittle: any refactor that callshandleSubmitsynchronously during render (e.g. from auseEffect) would throwReferenceError: Cannot access 'isCluster' before initialization, and ESLint'sno-use-before-definerule will flag it.♻️ Suggested ordering
const [expiresDate, setExpiresDate] = useState(''); const scopeOptions: { value: ComplianceScope; label: string; description: string }[] = [ // ... ]; + const isCluster = scope === 'kind-cluster' || scope === 'cluster'; + const handleSubmit = async () => { // ... }; - - const isCluster = scope === 'kind-cluster' || scope === 'cluster';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/GuidedComplianceExceptionForm.tsx` around lines 67 - 114, Move the isCluster binding so it is declared before handleSubmit (or compute it inside handleSubmit) to avoid accessing isCluster before initialization; specifically, relocate the const isCluster = scope === 'kind-cluster' || scope === 'cluster' so it appears above the handleSubmit function (or inline the same expression at the start of handleSubmit) and update any references in handleSubmit that use isCluster to use the now-available variable/expression.
73-86: The namespace scope semantic is correct and already documented with an inline comment.For
scope === 'namespace', thematchobject is intentionally left empty, relying on the resource'smetadata.namespaceto scope the exception — a standard Kubernetes pattern. The existing comment at line 79 ('namespace' and 'cluster' scopes: empty match — applies to all workloads in scope) adequately explains this behavior.To increase confidence in this non-obvious behavior, consider adding unit tests for the namespace-scoped exception creation to validate that the empty match is properly handled by the operator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/GuidedComplianceExceptionForm.tsx` around lines 73 - 86, The code intentionally leaves match empty for scope === 'namespace' (per the inline comment) but lacks automated verification; add unit tests that exercise the spec-construction logic in GuidedComplianceExceptionForm (the code path that builds the spec object including match, posture, expiresAt, and reason) to assert that when scope is 'namespace' the resulting spec.match is an empty object and that the operator/client code that consumes spec will scope by resource.metadata.namespace; also add a test for 'cluster' scope (empty match) and the other branches (workload and kind-cluster) to verify match.resources contains the expected kind/name entries and posture includes the controlID and action.src/exceptions/SecurityExceptions.tsx (1)
60-65: Endpoint URLs duplicated across files — consider centralizing.The same
/apis/kubescape.io/v1beta1/...securityexceptions[...]path templates are repeated here, inSecurityExceptionForm.tsx, and in each guided form. IfpluralName/group/version ever change inmodel.tsx, all of these have to be kept in sync manually. A small helper (e.g.securityExceptionUrl(row)/clusterSecurityExceptionUrl(name)) derived fromsecurityExceptionClass.apiEndpointwould remove that risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/SecurityExceptions.tsx` around lines 60 - 65, The duplicated hard-coded API paths for security exceptions should be replaced with central helper functions so changes to pluralName/group/version in model.tsx propagate automatically; add utilities like securityExceptionUrl(row) and clusterSecurityExceptionUrl(name) (derived from securityExceptionClass.apiEndpoint) and use them in SecurityExceptions.tsx (replace the inline `/apis/kubescape.io/v1beta1/...securityexceptions...` usages), SecurityExceptionForm.tsx, and all guided forms; update calls that currently pass row.name/row.namespace to instead call these helpers before calling remove() and anywhere else the endpoint string is constructed.src/exceptions/GuidedVulnerabilityExceptionForm.tsx (1)
119-144: DefineisClusterbeforehandleSubmitfor clarity.
handleSubmit(line 82) referencesisCluster(line 119) which is declared later at line 144. While this works at runtime becausehandleSubmitis invoked only after render completes (soisClusterhas been initialized in the closure), the inverted order makes the code harder to reason about and is fragile under future refactors that might move logic into the initialization phase.♻️ Proposed reorder
const [reason, setReason] = useState(''); const [expiresDate, setExpiresDate] = useState(''); + + const isCluster = scope === 'image-exact' || scope === 'image-glob'; const scopeOptions: { value: VulnScope; label: string; description: string }[] = [ @@ } }; - - const isCluster = scope === 'image-exact' || scope === 'image-glob'; return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/GuidedVulnerabilityExceptionForm.tsx` around lines 119 - 144, Move the isCluster computation so it appears before handleSubmit: currently handleSubmit references isCluster (and scope) but isCluster is defined later, which hurts readability and can break future refactors; relocate the const isCluster = scope === 'image-exact' || scope === 'image-glob' declaration above the handleSubmit function (and ensure any dependent closures still reference the same identifier) so handleSubmit sees clearly defined variables.src/exceptions/SecurityExceptionForm.tsx (2)
103-109:nameis sanitized only at initialization — user-entered values can produce invalid Kubernetes names.The initial value runs through
sanitizeName, but subsequent user edits in the NameTextFieldare stored verbatim. Submitting an invalid name (uppercase, dots, leading/trailing dashes, >63 chars) will only fail server-side with a generic API error, hurting UX. Consider validating or auto-sanitizing on change/blur and surfacing a client-side error before the request fires.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/SecurityExceptionForm.tsx` around lines 103 - 109, The Name state is only sanitized on initialization (useState) so edits via the Name TextField can create invalid Kubernetes names; update SecurityExceptionForm to sanitize and/or validate user input on change and/or blur: call sanitizeName inside the TextField onChange (or onBlur) before calling setName, and add a client-side validation using the same rules (max 63 chars, lowercase alphanumerics and dashes, no leading/trailing dash, no dots) to surface an inline error message and block submission in the form submit handler (where the API call is made) until the name passes validation.
257-275: Replace ternary-as-statement withif/else.The
isEdit ? await put(...) : await post(...)pattern uses a ternary expression purely for its side effect, which several ESLint configs (no-unused-expressions) flag, and it's slightly harder to read than a plain conditional. Consider:♻️ Proposed change
- const url = `/apis/kubescape.io/v1beta1/clustersecurityexceptions/${name}`; - isEdit - ? await put(url, obj as any) - : await post('/apis/kubescape.io/v1beta1/clustersecurityexceptions', obj); + if (isEdit) { + await put(`/apis/kubescape.io/v1beta1/clustersecurityexceptions/${name}`, obj as any); + } else { + await post('/apis/kubescape.io/v1beta1/clustersecurityexceptions', obj); + }And the matching block for the namespaced branch.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/SecurityExceptionForm.tsx` around lines 257 - 275, Replace the ternary expressions used for side effects (isEdit ? await put(...) : await post(...)) with explicit if/else blocks in both the cluster-scoped and namespaced branches inside SecurityExceptionForm (the code handling url, obj, isEdit, put and post): detect isEdit and call await put(url, obj as any) in the if branch, otherwise call await post(...) in the else branch, preserving the same url and obj values for each branch (including the cluster URL `/apis/kubescape.io/v1beta1/clustersecurityexceptions/${name}` and the namespaced URL `/apis/kubescape.io/v1beta1/namespaces/${namespace}/securityexceptions`).src/exceptions/shared.tsx (1)
239-245:toImageGlobmishandles digest references.For digest-pinned images such as
nginx@sha256:abc123…,lastIndexOf(':')lands inside the hash, so the function strips the digest and returnsnginx@sha256:*, which is not a meaningful pattern. Consider detecting@sha256:(or generally@) and either preserving the digest section or returning the repo with:*.♻️ Possible refinement
export function toImageGlob(imageRef: string): string { + // Strip digest reference, then operate on the tag form. + const at = imageRef.indexOf('@'); + const ref = at > 0 ? imageRef.substring(0, at) : imageRef; - const lastColon = imageRef.lastIndexOf(':'); - if (lastColon > 0 && !imageRef.substring(lastColon + 1).includes('/')) { - return imageRef.substring(0, lastColon) + ':*'; + const lastColon = ref.lastIndexOf(':'); + if (lastColon > 0 && !ref.substring(lastColon + 1).includes('/')) { + return ref.substring(0, lastColon) + ':*'; } - return imageRef + ':*'; + return ref + ':*'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/exceptions/shared.tsx` around lines 239 - 245, toImageGlob currently uses lastIndexOf(':') which misinterprets digest-pinned refs (e.g., nginx@sha256:...) and produces invalid patterns; update toImageGlob to first check for an '@' and if present return the part before '@' with ':*' appended (i.e., strip the digest and produce repo + ':*'), otherwise keep the existing tag-handling logic that checks lastIndexOf(':') and returns tag->':*' or imageRef + ':*' as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/compliance/ControlResults.tsx`:
- Around line 173-183: The Create Security Exception button currently renders
for every row; update the accessorFn for the IconButton (the lambda receiving
workloadScan: WorkloadConfigurationScanSummary and calling
setExceptionFormWorkload) to only render the actionable icon when the row is a
failed, non-excepted result — e.g. guard with workloadScan.status === 'Failed'
&& !workloadScan.exceptedByPolicy and otherwise return null (or a
disabled/hidden control). Mirror the same gating logic used by the Fix column so
users cannot create exceptions for Passed or already excepted rows.
In `@src/compliance/ResourceList.tsx`:
- Around line 49-54: The "Excluded" column in ResourceList.tsx uses Cell: ({
cell }: any) => (cell.getValue() ? 'Yes' : '') which leaves falsey values
ambiguous; update the Cell renderer for accessorKey 'exceptedByPolicy' to return
'Yes' or 'No' to match NamespaceView.tsx (i.e., render cell.getValue() ? 'Yes' :
'No'), ensuring consistent boolean display across ResourceList and NamespaceView
components.
In `@src/compliance/WorkloadScanDetails.tsx`:
- Around line 100-105: The current fallback to empty strings for workloadName,
workloadNamespace, and workloadKind can create invalid exception specs passed
into GuidedComplianceExceptionForm; instead, detect missing labels from
workloadConfigurationScan.metadata.labels (do not coerce to '') and add
validation logic to disable the action button (or surface a visible warning)
when any of workloadName, workloadNamespace, or workloadKind is absent,
preventing construction/submission of match.resources and the derived exception
name until all three labels are present.
In `@src/exceptions/GuidedClusterComplianceExceptionForm.tsx`:
- Line 32: The default name generated in the useState line (name / setName using
sanitizeName(`${controlID}-cluster`)) is deterministic and can collide; change
the initializer to append a short unique suffix (e.g., Date.now() or a 4-6 char
random/timestamp fragment) to sanitizeName(`${controlID}-cluster-<suffix>`) so
repeated cluster-wide exceptions get unique defaults, and update the form
submission code that performs the POST to detect HTTP 409 responses and show a
specific toast/error message indicating a name collision (suggesting the user
rename or auto-append a suffix) instead of the generic "Failed to create"
message.
- Around line 42-47: The current construction `${expiresDate}T00:00:00Z` sets
expiresAt to the UTC start of the chosen day which can already be in the past
for users east of UTC; change this to produce the UTC end-of-day for the
selected date (or convert the user's local end-of-day into UTC) and reuse it
across forms by adding a helper (e.g., endOfDayUtc(dateString) in shared.tsx).
Update GuidedClusterComplianceExceptionForm (the spec object using expiresAt),
GuidedClusterVulnerabilityExceptionForm, and GuidedComplianceExceptionForm to
call the new helper instead of embedding `${expiresDate}T00:00:00Z`. Ensure the
helper accepts the date string from the date picker and returns an ISO UTC
timestamp representing the last instant of that date (e.g., 23:59:59.999Z).
In `@src/exceptions/GuidedClusterVulnerabilityExceptionForm.tsx`:
- Around line 1-9: The import block in
GuidedClusterVulnerabilityExceptionForm.tsx is not ordered per the project's
lint/format rules; run the project's automatic fixer (npm run format or the
configured autofix) to sort and group imports so symbols like post,
Button/Dialog/DialogActions/DialogContent/DialogTitle/Stack, useSnackbar,
useState, VulnerabilityJustification, VulnerabilityStatus, ContextBadge,
MetadataFields, VulnExceptionFields, and sanitizeName appear in the canonical
import order enforced by CI.
In `@src/exceptions/SecurityExceptionDetail.tsx`:
- Around line 58-65: The current mapping in matchRows renders resource entries
as `${r.kind}/${r.name}`, producing "Kind/undefined" when r.name is absent;
update the mapping in SecurityExceptionDetail (the matchRows creation that
iterates spec.match.resources) to use a safe placeholder for missing names
(e.g., `${r.kind}/${r.name ?? '*'}` or `${r.kind}/${r.name ? r.name : '*'}`) so
rules that intentionally omit name (as created by SecurityExceptionForm) show a
wildcard instead of "undefined". Ensure the check uses r.name falsiness rather
than relying on presence so empty/undefined names render the wildcard.
In `@src/exceptions/SecurityExceptions.tsx`:
- Around line 57-77: The handleDelete function currently deletes a
SecurityException immediately; add a user confirmation step before calling
remove() to prevent accidental deletion (e.g., use window.confirm or open an MUI
confirmation dialog and abort if the user cancels). Locate handleDelete and wrap
the deletion branches (the calls to remove(...) for cluster and namespaced CRs)
behind the confirmation check; only proceed to call remove(...), update state
via setClusterList / setNamespacedList, and call enqueueSnackbar on success when
the user confirms. Ensure the cancel path does nothing (no API call) and
optionally shows no snackbar or shows a cancelled/info snackbar.
In `@src/exceptions/shared.tsx`:
- Around line 230-237: sanitizeName may end with a trailing '-' if substring(0,
63) cuts off a name at a dash; fix by re-applying the leading/trailing dash trim
after truncation so the final string always starts/ends with [a-z0-9]. In
function sanitizeName, perform lowercase, replace invalid chars and collapse
dashes, then apply substring(0, 63) and immediately run the trim regex
replaceAll(/^-|-$/g) again (and if desired handle an empty result case),
ensuring the final returned value conforms to DNS-1123 (starts/ends with
a-z0-9).
In `@src/index.tsx`:
- Around line 38-40: The two routes SecurityExceptionDetail and
SecurityExceptionClusterDetail overlap because
'/kubescape/exceptions/:namespace/:name' will match 'cluster' as a namespace and
shadow '/kubescape/exceptions/cluster/:name'; change one path to make them
disambiguated (for example rename SecurityExceptionDetail to use a prefix like
'/kubescape/exceptions/ns/:namespace/:name' or rename the cluster route to
'/kubescape/exceptions/_cluster/:name') and then update all
link/navigation/route-generation sites that reference SecurityExceptionDetail or
SecurityExceptionClusterDetail to use the new string constants so namespaced
exceptions in a namespace named "cluster" are reachable.
In `@src/vulnerabilities/ResourceList.tsx`:
- Line 4: Remove the unused import by deleting the import { Icon } from
'@iconify/react' statement, or if an icon was intended, add a concrete usage of
Icon in the ResourceList/ResourceRow JSX (reference the Icon symbol where
needed) so that Icon is actually referenced; then run the linter to confirm the
"'Icon' is defined but never used" error is resolved.
---
Outside diff comments:
In `@src/compliance/Compliance.tsx`:
- Around line 117-139: fetchData is being called fire-and-forget so any thrown
error (from applySecurityExceptionsToWorkloadScans or JSON.parse inside
fetchCustomFrameworks) will leave setLoading stuck and provide no UI feedback;
wrap the fetchData invocation with a proper promise chain (e.g., call
fetchData().catch(err => { setError(err); }).finally(() => setLoading(false)))
or otherwise handle errors returned by fetchData, add an error state (e.g.,
setError) to surface the message similar to Vulnerabilities.tsx, and ensure
setLoading is cleared in a finally block; specifically reference fetchData,
applySecurityExceptionsToWorkloadScans, fetchCustomFrameworks,
fetchSecurityExceptions and setLoading when making these changes so the async
error paths are caught and UI state is updated.
---
Nitpick comments:
In `@src/compliance/Compliance.tsx`:
- Around line 12-24: Imports in Compliance.tsx are misordered: the named import
Icon from '@iconify/react' is interleaved with the `@mui/material` imports and
GuidedClusterComplianceExceptionForm is placed before sibling relative imports;
run the project's import-sort autofix (or re-run the configured
linter/import-sort) to reorder imports so external packages (e.g.,
'@iconify/react') group together, then `@mui/material` block follows, and local
relative imports (including GuidedClusterComplianceExceptionForm and other
../common/... imports) appear last; ensure the symbols Icon and
GuidedClusterComplianceExceptionForm remain unchanged while fixing import order.
In `@src/compliance/NamespaceView.tsx`:
- Around line 33-40: Remove the now-unused optional prop setWorkloadScanData
from the NamespaceView component's props type and update any callers to stop
passing it (e.g., remove setWorkloadScanData={setWorkloadScanData} in
Compliance.tsx). Specifically, edit the NamespaceView declaration to only accept
workloadScanData: WorkloadConfigurationScanSummary[] | null and remove
setWorkloadScanData from the Readonly prop shape, then remove the corresponding
prop usage in Compliance.tsx where it is passed through.
In `@src/compliance/ResourceList.tsx`:
- Around line 10-20: The prop type for KubescapeWorkloadConfigurationScanList
still includes the unused setWorkloadScanData; remove setWorkloadScanData from
the component props type declaration and from any places that pass it in (e.g.,
the caller in Compliance.tsx around where KubescapeWorkloadConfigurationScanList
is instantiated) so the prop surface matches actual usage; update the Readonly<{
... }> definition inside KubescapeWorkloadConfigurationScanList (and remove any
passing of setWorkloadScanData in Compliance.tsx) to eliminate the unused prop.
In `@src/exceptions/GuidedComplianceExceptionForm.tsx`:
- Around line 67-114: Move the isCluster binding so it is declared before
handleSubmit (or compute it inside handleSubmit) to avoid accessing isCluster
before initialization; specifically, relocate the const isCluster = scope ===
'kind-cluster' || scope === 'cluster' so it appears above the handleSubmit
function (or inline the same expression at the start of handleSubmit) and update
any references in handleSubmit that use isCluster to use the now-available
variable/expression.
- Around line 73-86: The code intentionally leaves match empty for scope ===
'namespace' (per the inline comment) but lacks automated verification; add unit
tests that exercise the spec-construction logic in GuidedComplianceExceptionForm
(the code path that builds the spec object including match, posture, expiresAt,
and reason) to assert that when scope is 'namespace' the resulting spec.match is
an empty object and that the operator/client code that consumes spec will scope
by resource.metadata.namespace; also add a test for 'cluster' scope (empty
match) and the other branches (workload and kind-cluster) to verify
match.resources contains the expected kind/name entries and posture includes the
controlID and action.
In `@src/exceptions/GuidedVulnerabilityExceptionForm.tsx`:
- Around line 119-144: Move the isCluster computation so it appears before
handleSubmit: currently handleSubmit references isCluster (and scope) but
isCluster is defined later, which hurts readability and can break future
refactors; relocate the const isCluster = scope === 'image-exact' || scope ===
'image-glob' declaration above the handleSubmit function (and ensure any
dependent closures still reference the same identifier) so handleSubmit sees
clearly defined variables.
In `@src/exceptions/SecurityExceptionForm.tsx`:
- Around line 103-109: The Name state is only sanitized on initialization
(useState) so edits via the Name TextField can create invalid Kubernetes names;
update SecurityExceptionForm to sanitize and/or validate user input on change
and/or blur: call sanitizeName inside the TextField onChange (or onBlur) before
calling setName, and add a client-side validation using the same rules (max 63
chars, lowercase alphanumerics and dashes, no leading/trailing dash, no dots) to
surface an inline error message and block submission in the form submit handler
(where the API call is made) until the name passes validation.
- Around line 257-275: Replace the ternary expressions used for side effects
(isEdit ? await put(...) : await post(...)) with explicit if/else blocks in both
the cluster-scoped and namespaced branches inside SecurityExceptionForm (the
code handling url, obj, isEdit, put and post): detect isEdit and call await
put(url, obj as any) in the if branch, otherwise call await post(...) in the
else branch, preserving the same url and obj values for each branch (including
the cluster URL `/apis/kubescape.io/v1beta1/clustersecurityexceptions/${name}`
and the namespaced URL
`/apis/kubescape.io/v1beta1/namespaces/${namespace}/securityexceptions`).
In `@src/exceptions/SecurityExceptions.tsx`:
- Around line 60-65: The duplicated hard-coded API paths for security exceptions
should be replaced with central helper functions so changes to
pluralName/group/version in model.tsx propagate automatically; add utilities
like securityExceptionUrl(row) and clusterSecurityExceptionUrl(name) (derived
from securityExceptionClass.apiEndpoint) and use them in SecurityExceptions.tsx
(replace the inline `/apis/kubescape.io/v1beta1/...securityexceptions...`
usages), SecurityExceptionForm.tsx, and all guided forms; update calls that
currently pass row.name/row.namespace to instead call these helpers before
calling remove() and anywhere else the endpoint string is constructed.
In `@src/exceptions/shared.tsx`:
- Around line 239-245: toImageGlob currently uses lastIndexOf(':') which
misinterprets digest-pinned refs (e.g., nginx@sha256:...) and produces invalid
patterns; update toImageGlob to first check for an '@' and if present return the
part before '@' with ':*' appended (i.e., strip the digest and produce repo +
':*'), otherwise keep the existing tag-handling logic that checks
lastIndexOf(':') and returns tag->':*' or imageRef + ':*' as before.
In `@src/softwarecomposition/SecurityException.ts`:
- Around line 9-14: The matchExpressions fields in SecurityExceptionMatchSpec
currently use any[] which loses type safety; define a small interface (e.g.,
LabelSelectorRequirement with properties key: string; operator: 'In' | 'NotIn' |
'Exists' | 'DoesNotExist'; values?: string[]) and replace matchExpressions?:
any[] with matchExpressions?: LabelSelectorRequirement[]; update the two
selector shapes (namespaceSelector and objectSelector) to reference this typed
matchExpressions so form components and detail views can rely on the concrete
type.
- Around line 53-74: The CLAUDE.md guideline currently mandates using
spdx.softwarecomposition.kubescape.io/v1beta1 for all .ts files but the code
defines CRDs SecurityException and ClusterSecurityException with apiVersion
'kubescape.io/v1beta1'; update CLAUDE.md to explicitly allow
kubescape.io/v1beta1 for the SecurityException and ClusterSecurityException CRDs
(or broaden the rule to accept authorized operator CRDs beyond spdx.*) so future
reviews don't flag SecurityException and ClusterSecurityException; mention the
two symbols by name and the allowed apiVersion so the guideline matches the
implementation.
In `@src/vulnerabilities/Vulnerabilities.tsx`:
- Around line 4-43: The import ordering breaks the third-party-then-relative
grouping by placing GuidedClusterVulnerabilityExceptionForm between
`@mui/material` and React imports; move the
GuidedClusterVulnerabilityExceptionForm import so all third-party imports (e.g.,
'@mui/material', 'react', '@iconify/react', etc.) remain grouped together and
place GuidedClusterVulnerabilityExceptionForm with the other project-relative
imports (near VulnerabilityManifest, ImageListView, WorkloadScanListView) or run
the project's import-sort/autofix to restore the expected grouping.
In `@src/vulnerabilities/WorkloadScanDetails.tsx`:
- Around line 97-105: Wrap the props type for the Matches component in
Readonly<> to match repository convention (i.e., change the parameter type from
the inline object to Readonly<{ ... }>) so the props are treated as immutable;
keep the property names and types the same (manifest, relevant, workloadName,
workloadNamespace, workloadKind, imageRef) and leave the existing destructuring
in the Matches function body unchanged.
🪄 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: a0ab434c-6e05-4344-8b39-8f62d589debd
📒 Files selected for processing (30)
src/common/PluginHelper.tssrc/common/config-store.tsxsrc/compliance/Compliance.tsxsrc/compliance/ControlResults.tsxsrc/compliance/NamespaceView.tsxsrc/compliance/ResourceList.tsxsrc/compliance/WorkloadScanDetails.tsxsrc/exceptions/ConfirmDialog.tsxsrc/exceptions/ExceptionDialog.tsxsrc/exceptions/ExceptionGroup.tsxsrc/exceptions/ExceptionGroups.tsxsrc/exceptions/ExceptionPolicy.tssrc/exceptions/GuidedClusterComplianceExceptionForm.tsxsrc/exceptions/GuidedClusterVulnerabilityExceptionForm.tsxsrc/exceptions/GuidedComplianceExceptionForm.tsxsrc/exceptions/GuidedVulnerabilityExceptionForm.tsxsrc/exceptions/SecurityExceptionDetail.tsxsrc/exceptions/SecurityExceptionForm.tsxsrc/exceptions/SecurityExceptions.tsxsrc/exceptions/apply-exceptions.tssrc/exceptions/apply-security-exceptions.tssrc/exceptions/mutate-exception.tssrc/exceptions/shared.tsxsrc/index.tsxsrc/model.tsxsrc/softwarecomposition/SecurityException.tssrc/vulnerabilities/CVEResults.tsxsrc/vulnerabilities/ResourceList.tsxsrc/vulnerabilities/Vulnerabilities.tsxsrc/vulnerabilities/WorkloadScanDetails.tsx
💤 Files with no reviewable changes (8)
- src/common/config-store.tsx
- src/exceptions/ExceptionGroups.tsx
- src/exceptions/ConfirmDialog.tsx
- src/exceptions/ExceptionDialog.tsx
- src/exceptions/ExceptionPolicy.ts
- src/exceptions/mutate-exception.ts
- src/exceptions/ExceptionGroup.tsx
- src/exceptions/apply-exceptions.ts
| const { enqueueSnackbar } = useSnackbar(); | ||
|
|
||
| const [action, setAction] = useState<'ignore' | 'alert_only'>('ignore'); | ||
| const [name, setName] = useState(sanitizeName(`${controlID}-cluster`)); |
There was a problem hiding this comment.
Default name collides for repeated cluster-wide exceptions on the same control.
sanitizeName(\${controlID}-cluster`)` is deterministic, so a second cluster-wide exception for the same control gets the same default name and POST will fail with 409 AlreadyExists. The user does see a generic "Failed to create" toast but the cause is opaque. Consider appending a short timestamp/suffix or detecting 409 and surfacing a more specific message.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/exceptions/GuidedClusterComplianceExceptionForm.tsx` at line 32, The
default name generated in the useState line (name / setName using
sanitizeName(`${controlID}-cluster`)) is deterministic and can collide; change
the initializer to append a short unique suffix (e.g., Date.now() or a 4-6 char
random/timestamp fragment) to sanitizeName(`${controlID}-cluster-<suffix>`) so
repeated cluster-wide exceptions get unique defaults, and update the form
submission code that performs the POST to detect HTTP 409 responses and show a
specific toast/error message indicating a name collision (suggesting the user
rename or auto-append a suffix) instead of the generic "Failed to create"
message.
Comment on lines
+42
to
+47
| const spec = { | ||
| ...(reason && { reason }), | ||
| ...(expiresDate && { expiresAt: `${expiresDate}T00:00:00Z` }), | ||
| match: {}, | ||
| posture: [{ controlID, action }], | ||
| }; |
There was a problem hiding this comment.
expiresAt is midnight UTC of the picked date — can already be in the past.
${expiresDate}T00:00:00Z produces the start of the chosen day in UTC. For a user east of UTC (e.g. CET/IST/JST) selecting "today", expiresAt is several hours in the past at submit time, so the operator-side expiry check effectively rejects/expires the exception immediately. End-of-day in UTC (or, ideally, end-of-day in the user's local timezone serialized to UTC) matches what date pickers conventionally imply.
The same pattern exists in GuidedClusterVulnerabilityExceptionForm.tsx and GuidedComplianceExceptionForm.tsx; consider extracting a helper in shared.tsx and fixing it in one place.
🕒 Suggested fix
- ...(expiresDate && { expiresAt: `${expiresDate}T00:00:00Z` }),
+ ...(expiresDate && { expiresAt: `${expiresDate}T23:59:59Z` }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/exceptions/GuidedClusterComplianceExceptionForm.tsx` around lines 42 -
47, The current construction `${expiresDate}T00:00:00Z` sets expiresAt to the
UTC start of the chosen day which can already be in the past for users east of
UTC; change this to produce the UTC end-of-day for the selected date (or convert
the user's local end-of-day into UTC) and reuse it across forms by adding a
helper (e.g., endOfDayUtc(dateString) in shared.tsx). Update
GuidedClusterComplianceExceptionForm (the spec object using expiresAt),
GuidedClusterVulnerabilityExceptionForm, and GuidedComplianceExceptionForm to
call the new helper instead of embedding `${expiresDate}T00:00:00Z`. Ensure the
helper accepts the date string from the date picker and returns an ISO UTC
timestamp representing the last instant of that date (e.g., 23:59:59.999Z).
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/compliance/WorkloadScanDetails.tsx (1)
100-208:⚠️ Potential issue | 🟡 MinorEmpty-string fallbacks still flow into the guided form (unchanged from previous review).
The
?? ''defaults on Lines 100-105 still allowworkloadName/workloadNamespace/workloadKindto silently become"", which then propagate into the POST URL (/namespaces//securityexceptions) and the constructedmatch.resources[0]inGuidedComplianceExceptionForm. Disabling the IconButton when any label is absent is more robust than coercing.🛡️ Suggested guard
- const workloadName = - workloadConfigurationScan.metadata.labels['kubescape.io/workload-name'] ?? ''; - const workloadNamespace = - workloadConfigurationScan.metadata.labels['kubescape.io/workload-namespace'] ?? ''; - const workloadKind = - workloadConfigurationScan.metadata.labels['kubescape.io/workload-kind'] ?? ''; + const workloadName = workloadConfigurationScan.metadata.labels['kubescape.io/workload-name']; + const workloadNamespace = + workloadConfigurationScan.metadata.labels['kubescape.io/workload-namespace']; + const workloadKind = workloadConfigurationScan.metadata.labels['kubescape.io/workload-kind']; + const canCreateException = !!(workloadName && workloadNamespace && workloadKind); @@ - <Tooltip title="Create Security Exception"> - <IconButton size="small" onClick={() => setExceptionControl(control)}> + <Tooltip + title={ + canCreateException + ? 'Create Security Exception' + : 'Workload labels missing — cannot create exception' + } + > + <IconButton + size="small" + disabled={!canCreateException} + onClick={() => setExceptionControl(control)} + > <Icon icon="mdi:shield-plus-outline" /> </IconButton> </Tooltip>(
workloadName!/etc. or non-null assertions in the form props block can be added once typed accordingly.)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/compliance/WorkloadScanDetails.tsx` around lines 100 - 208, The current code coerces workloadName/workloadNamespace/workloadKind to empty strings (workloadName = ... ?? '') which lets empty values flow into GuidedComplianceExceptionForm; instead remove the "?? ''" fallbacks so the variables can be undefined/null, and disable the "Create Security Exception" IconButton (the IconButton that calls setExceptionControl) unless all three (workloadName, workloadNamespace, workloadKind) are present (e.g. disabled={!workloadName || !workloadNamespace || !workloadKind}); also defensively avoid calling setExceptionControl when any of those values are missing and keep GuidedComplianceExceptionForm usage unchanged (it will only render when exceptionControl is set).
🧹 Nitpick comments (2)
src/compliance/Compliance.tsx (1)
124-130: Consider logging the swallowed exceptions error.The
.catch(() => …)discards the error silently.fetchSecurityExceptionsalready has per-request fallbacks internally, so this outer catch only fires on unexpected failures — exactly the cases worth logging. Aconsole.errorhere would aid debugging without affecting the temporary fallback behavior.♻️ Suggested change
// TEMPORARY: client-side exception apply — remove once operator handles this at scan time - const exceptions = await fetchSecurityExceptions().catch(() => ({ - namespaced: [], - cluster: [], - namespaceLabelsByName: new Map(), - })); + const exceptions = await fetchSecurityExceptions().catch(error => { + console.error('Failed to fetch security exceptions', error); + return { + namespaced: [], + cluster: [], + namespaceLabelsByName: new Map(), + }; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/compliance/Compliance.tsx` around lines 124 - 130, The catch on fetchSecurityExceptions currently swallows errors; change it to log the caught error (e.g., via console.error or processLogger.error) while still returning the fallback object so the temporary client-side exception apply behaviour remains. Specifically, update the promise chain around fetchSecurityExceptions() (the block that assigns exceptions used by applySecurityExceptionsToWorkloadScans and configurationScanContext.workloadScans) to accept the error parameter in the catch, log a descriptive message including that error, and then return the same fallback shape.src/vulnerabilities/WorkloadScanDetails.tsx (1)
197-207: Optional: preferCelloveraccessorFnfor JSX rendering.For consistency with the other columns in this table (lines 141, 195) and standard
material-react-tablesemantics, consider keepingaccessorFnfor the underlying value and moving the JSX toCell. As-is the column "value" is a React element which would break sorting/filtering if ever enabled. Functionally fine for an empty-header action column today.♻️ Suggested refactor
{ header: '', - accessorFn: (item: VulnerabilityManifest.Match) => ( - <Tooltip title="Create Vulnerability Exception"> - <IconButton size="small" onClick={() => setSelectedCve(item.vulnerability.id)}> - <Icon icon="mdi:shield-plus-outline" /> - </IconButton> - </Tooltip> - ), + accessorFn: (item: VulnerabilityManifest.Match) => item.vulnerability.id, + Cell: ({ row }: any) => ( + <Tooltip title="Create Vulnerability Exception"> + <IconButton + size="small" + onClick={() => setSelectedCve(row.original.vulnerability.id)} + > + <Icon icon="mdi:shield-plus-outline" /> + </IconButton> + </Tooltip> + ), gridTemplate: 'min-content', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/vulnerabilities/WorkloadScanDetails.tsx` around lines 197 - 207, The action column currently returns a React element from accessorFn which prevents using the column value for sorting/filtering; change the column so accessorFn returns the underlying value (e.g., the CVE id from the VulnerabilityManifest.Match item.vulnerability.id) and add a Cell renderer that builds the JSX action button and calls setSelectedCve using the accessor value (via row.getValue() or row.original). Update the column object that currently references VulnerabilityManifest.Match and setSelectedCve so accessorFn -> id and Cell -> Tooltip/IconButton JSX using that id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/compliance/Compliance.tsx`:
- Around line 422-444: fetchCustomFrameworks currently calls
JSON.parse(configMap.data.controlsIDs) directly which will throw on missing or
malformed controlsIDs and abort the whole load; update the per-configMap
handling inside fetchCustomFrameworks to safely parse controlsIDs (check
configMap.data?.controlsIDs is a string, wrap JSON.parse in a try/catch or use a
safeParse helper) and fall back to an empty array on error while logging the
parse failure, then continue building the FrameWork object and pushing it to
customFrameworks before finally calling setCustomFrameworks; target the
JSON.parse call and the loop over response?.items (configMap) to implement this
defensive parsing.
In `@src/exceptions/apply-security-exceptions.ts`:
- Around line 49-56: The loop over selector.matchExpressions can throw when
expr.values is undefined for 'In'/'NotIn'; update the checks in that loop (the
block inspecting selector.matchExpressions, expr.operator, expr.key and
expr.values) to guard against missing values by verifying
Array.isArray(expr.values) before calling .includes; if values is missing or not
an array treat the expression as non-matching (return false) so a malformed
LabelSelectorRequirement will be safely skipped rather than crashing the
compliance view.
In `@src/exceptions/GuidedComplianceExceptionForm.tsx`:
- Around line 69-114: The handleSubmit function can POST to a namespaced URL
using workloadNamespace even when it's empty, causing bad requests; before
building spec/issuing post in handleSubmit, validate workloadNamespace whenever
scope !== 'cluster' (or when isCluster is false and the URL includes
${workloadNamespace}); if empty/invalid, show an error via enqueueSnackbar and
return early. Update checks around scope/isCluster and the post call sites (the
branches that construct `/namespaces/${workloadNamespace}/securityexceptions`)
to ensure workloadNamespace is present and non-empty before calling post.
In `@src/exceptions/SecurityExceptionForm.tsx`:
- Around line 222-246: The form allows saving vulnerabilities with status
"not_affected" without a justification; update the submit-path that builds the
spec (the logic producing validVulns and the final spec) to enforce that any
vuln entry with status === 'not_affected' must have a non-empty justification:
validate vulnEntries (or validVulns) before mapping—if any entry violates this,
surface a validation error/abort submit (same mechanism used elsewhere in the
form) rather than including the entry in the spec; adjust the code that
constructs vulnerabilities (the map producing VulnerabilityException[]) to
assume justification exists for not_affected entries only after the prior
validation passes.
---
Duplicate comments:
In `@src/compliance/WorkloadScanDetails.tsx`:
- Around line 100-208: The current code coerces
workloadName/workloadNamespace/workloadKind to empty strings (workloadName = ...
?? '') which lets empty values flow into GuidedComplianceExceptionForm; instead
remove the "?? ''" fallbacks so the variables can be undefined/null, and disable
the "Create Security Exception" IconButton (the IconButton that calls
setExceptionControl) unless all three (workloadName, workloadNamespace,
workloadKind) are present (e.g. disabled={!workloadName || !workloadNamespace ||
!workloadKind}); also defensively avoid calling setExceptionControl when any of
those values are missing and keep GuidedComplianceExceptionForm usage unchanged
(it will only render when exceptionControl is set).
---
Nitpick comments:
In `@src/compliance/Compliance.tsx`:
- Around line 124-130: The catch on fetchSecurityExceptions currently swallows
errors; change it to log the caught error (e.g., via console.error or
processLogger.error) while still returning the fallback object so the temporary
client-side exception apply behaviour remains. Specifically, update the promise
chain around fetchSecurityExceptions() (the block that assigns exceptions used
by applySecurityExceptionsToWorkloadScans and
configurationScanContext.workloadScans) to accept the error parameter in the
catch, log a descriptive message including that error, and then return the same
fallback shape.
In `@src/vulnerabilities/WorkloadScanDetails.tsx`:
- Around line 197-207: The action column currently returns a React element from
accessorFn which prevents using the column value for sorting/filtering; change
the column so accessorFn returns the underlying value (e.g., the CVE id from the
VulnerabilityManifest.Match item.vulnerability.id) and add a Cell renderer that
builds the JSX action button and calls setSelectedCve using the accessor value
(via row.getValue() or row.original). Update the column object that currently
references VulnerabilityManifest.Match and setSelectedCve so accessorFn -> id
and Cell -> Tooltip/IconButton JSX using that id.
🪄 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: 491bb821-1d74-4c04-9123-e979255323d6
📒 Files selected for processing (31)
CLAUDE.mdsrc/common/PluginHelper.tssrc/common/config-store.tsxsrc/compliance/Compliance.tsxsrc/compliance/ControlResults.tsxsrc/compliance/NamespaceView.tsxsrc/compliance/ResourceList.tsxsrc/compliance/WorkloadScanDetails.tsxsrc/exceptions/ConfirmDialog.tsxsrc/exceptions/ExceptionDialog.tsxsrc/exceptions/ExceptionGroup.tsxsrc/exceptions/ExceptionGroups.tsxsrc/exceptions/ExceptionPolicy.tssrc/exceptions/GuidedClusterComplianceExceptionForm.tsxsrc/exceptions/GuidedClusterVulnerabilityExceptionForm.tsxsrc/exceptions/GuidedComplianceExceptionForm.tsxsrc/exceptions/GuidedVulnerabilityExceptionForm.tsxsrc/exceptions/SecurityExceptionDetail.tsxsrc/exceptions/SecurityExceptionForm.tsxsrc/exceptions/SecurityExceptions.tsxsrc/exceptions/apply-exceptions.tssrc/exceptions/apply-security-exceptions.tssrc/exceptions/mutate-exception.tssrc/exceptions/shared.tsxsrc/index.tsxsrc/model.tsxsrc/softwarecomposition/SecurityException.tssrc/vulnerabilities/CVEResults.tsxsrc/vulnerabilities/ResourceList.tsxsrc/vulnerabilities/Vulnerabilities.tsxsrc/vulnerabilities/WorkloadScanDetails.tsx
💤 Files with no reviewable changes (8)
- src/common/config-store.tsx
- src/exceptions/ExceptionDialog.tsx
- src/exceptions/ConfirmDialog.tsx
- src/exceptions/ExceptionPolicy.ts
- src/exceptions/ExceptionGroup.tsx
- src/exceptions/mutate-exception.ts
- src/exceptions/apply-exceptions.ts
- src/exceptions/ExceptionGroups.tsx
✅ Files skipped from review due to trivial changes (4)
- src/vulnerabilities/ResourceList.tsx
- CLAUDE.md
- src/common/PluginHelper.ts
- src/model.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- src/exceptions/GuidedClusterComplianceExceptionForm.tsx
- src/softwarecomposition/SecurityException.ts
- src/exceptions/GuidedClusterVulnerabilityExceptionForm.tsx
- src/exceptions/SecurityExceptionDetail.tsx
- src/exceptions/GuidedVulnerabilityExceptionForm.tsx
- src/index.tsx
Comment on lines
+49
to
+56
| for (const expr of selector.matchExpressions ?? []) { | ||
| const has = expr.key in labels; | ||
| const val = labels[expr.key]; | ||
| if (expr.operator === 'Exists' && !has) return false; | ||
| if (expr.operator === 'DoesNotExist' && has) return false; | ||
| if (expr.operator === 'In' && !expr.values.includes(val)) return false; | ||
| if (expr.operator === 'NotIn' && expr.values.includes(val)) return false; | ||
| } |
There was a problem hiding this comment.
Potential TypeError when matchExpressions[*].values is omitted.
LabelSelectorRequirement.values is optional in the type definition (src/softwarecomposition/SecurityException.ts). For In/NotIn operators a malformed/partial CR will crash this loop with Cannot read properties of undefined (reading 'includes'), taking down the whole compliance view rather than just skipping that exception.
🛡️ Suggested guard
for (const expr of selector.matchExpressions ?? []) {
const has = expr.key in labels;
const val = labels[expr.key];
if (expr.operator === 'Exists' && !has) return false;
if (expr.operator === 'DoesNotExist' && has) return false;
- if (expr.operator === 'In' && !expr.values.includes(val)) return false;
- if (expr.operator === 'NotIn' && expr.values.includes(val)) return false;
+ if (expr.operator === 'In' && !(expr.values ?? []).includes(val)) return false;
+ if (expr.operator === 'NotIn' && (expr.values ?? []).includes(val)) return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| for (const expr of selector.matchExpressions ?? []) { | |
| const has = expr.key in labels; | |
| const val = labels[expr.key]; | |
| if (expr.operator === 'Exists' && !has) return false; | |
| if (expr.operator === 'DoesNotExist' && has) return false; | |
| if (expr.operator === 'In' && !expr.values.includes(val)) return false; | |
| if (expr.operator === 'NotIn' && expr.values.includes(val)) return false; | |
| } | |
| for (const expr of selector.matchExpressions ?? []) { | |
| const has = expr.key in labels; | |
| const val = labels[expr.key]; | |
| if (expr.operator === 'Exists' && !has) return false; | |
| if (expr.operator === 'DoesNotExist' && has) return false; | |
| if (expr.operator === 'In' && !(expr.values ?? []).includes(val)) return false; | |
| if (expr.operator === 'NotIn' && (expr.values ?? []).includes(val)) return false; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/exceptions/apply-security-exceptions.ts` around lines 49 - 56, The loop
over selector.matchExpressions can throw when expr.values is undefined for
'In'/'NotIn'; update the checks in that loop (the block inspecting
selector.matchExpressions, expr.operator, expr.key and expr.values) to guard
against missing values by verifying Array.isArray(expr.values) before calling
.includes; if values is missing or not an array treat the expression as
non-matching (return false) so a malformed LabelSelectorRequirement will be
safely skipped rather than crashing the compliance view.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vulnerabilities/CVEResults.tsx (1)
27-31:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winGuard empty filtered results before indexing.
Line 27 checks truthiness of an array, so it never catches the empty case. Array.prototype.filter() always returns an array (even when empty), making the check
if (!workloadScansFiltered)ineffective. Then line 30 accessesworkloadScansFiltered[0].imageScanwithout optional chaining on the index, which crashes when the array is empty.Suggested fix
- if (!workloadScansFiltered) { + if (workloadScansFiltered.length === 0) { return <></>; } - const firstCVE = workloadScansFiltered[0].imageScan?.matches?.find( + const firstCVE = workloadScansFiltered[0]?.imageScan?.matches?.find( match => match.vulnerability.id === cve );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vulnerabilities/CVEResults.tsx` around lines 27 - 31, workloadScansFiltered is being treated as nullable but filter() always returns an array, so guard against empty arrays and avoid direct indexing that can throw; before computing firstCVE return early when workloadScansFiltered.length === 0 (or use a falsy/empty check) and change the access to use optional chaining on the first element (e.g., workloadScansFiltered[0]?.imageScan?.matches?.find(...)) so that firstCVE resolution is safe when the array is empty or the first item/imageScan is undefined; update any related downstream use of firstCVE accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/vulnerabilities/CVEResults.tsx`:
- Around line 132-135: The IconButton inside the Tooltip (rendering Icon and
calling setExceptionFormWorkload) lacks an accessible name; add an explicit
accessible label (for example aria-label="Create Security Exception" or
aria-labelledby pointing to a hidden label) to the IconButton so screen readers
can discover the action while keeping the Tooltip for visual users.
---
Outside diff comments:
In `@src/vulnerabilities/CVEResults.tsx`:
- Around line 27-31: workloadScansFiltered is being treated as nullable but
filter() always returns an array, so guard against empty arrays and avoid direct
indexing that can throw; before computing firstCVE return early when
workloadScansFiltered.length === 0 (or use a falsy/empty check) and change the
access to use optional chaining on the first element (e.g.,
workloadScansFiltered[0]?.imageScan?.matches?.find(...)) so that firstCVE
resolution is safe when the array is empty or the first item/imageScan is
undefined; update any related downstream use of firstCVE accordingly.
🪄 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: 879a8d18-a5d6-48b8-8178-d0a999f4670a
📒 Files selected for processing (30)
CLAUDE.mdsrc/common/config-store.tsxsrc/compliance/Compliance.tsxsrc/compliance/ControlResults.tsxsrc/compliance/NamespaceView.tsxsrc/compliance/ResourceList.tsxsrc/compliance/WorkloadScanDetails.tsxsrc/exceptions/ConfirmDialog.tsxsrc/exceptions/ExceptionDialog.tsxsrc/exceptions/ExceptionGroup.tsxsrc/exceptions/ExceptionGroups.tsxsrc/exceptions/ExceptionPolicy.tssrc/exceptions/GuidedClusterComplianceExceptionForm.tsxsrc/exceptions/GuidedClusterVulnerabilityExceptionForm.tsxsrc/exceptions/GuidedComplianceExceptionForm.tsxsrc/exceptions/GuidedVulnerabilityExceptionForm.tsxsrc/exceptions/SecurityExceptionDetail.tsxsrc/exceptions/SecurityExceptionForm.tsxsrc/exceptions/SecurityExceptions.tsxsrc/exceptions/apply-exceptions.tssrc/exceptions/apply-security-exceptions.tssrc/exceptions/mutate-exception.tssrc/exceptions/shared.tsxsrc/index.tsxsrc/model.tsxsrc/softwarecomposition/SecurityException.tssrc/vulnerabilities/CVEResults.tsxsrc/vulnerabilities/ResourceList.tsxsrc/vulnerabilities/Vulnerabilities.tsxsrc/vulnerabilities/WorkloadScanDetails.tsx
💤 Files with no reviewable changes (8)
- src/common/config-store.tsx
- src/exceptions/ConfirmDialog.tsx
- src/exceptions/ExceptionGroup.tsx
- src/exceptions/ExceptionPolicy.ts
- src/exceptions/apply-exceptions.ts
- src/exceptions/ExceptionGroups.tsx
- src/exceptions/mutate-exception.ts
- src/exceptions/ExceptionDialog.tsx
✅ Files skipped from review due to trivial changes (2)
- src/vulnerabilities/ResourceList.tsx
- CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (18)
- src/compliance/ResourceList.tsx
- src/softwarecomposition/SecurityException.ts
- src/exceptions/GuidedClusterVulnerabilityExceptionForm.tsx
- src/compliance/WorkloadScanDetails.tsx
- src/exceptions/shared.tsx
- src/exceptions/GuidedComplianceExceptionForm.tsx
- src/compliance/NamespaceView.tsx
- src/vulnerabilities/Vulnerabilities.tsx
- src/exceptions/GuidedVulnerabilityExceptionForm.tsx
- src/exceptions/SecurityExceptions.tsx
- src/exceptions/SecurityExceptionDetail.tsx
- src/vulnerabilities/WorkloadScanDetails.tsx
- src/index.tsx
- src/exceptions/apply-security-exceptions.ts
- src/exceptions/SecurityExceptionForm.tsx
- src/compliance/ControlResults.tsx
- src/model.tsx
- src/compliance/Compliance.tsx
Comment on lines
+132
to
+135
| <Tooltip title="Create Security Exception"> | ||
| <IconButton size="small" onClick={() => setExceptionFormWorkload(workload)}> | ||
| <Icon icon="mdi:shield-plus-outline" /> | ||
| </IconButton> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file to see the current state at lines 132-135
cat -n src/vulnerabilities/CVEResults.tsx | sed -n '125,145p'Repository: kubescape/headlamp-plugin
Length of output: 973
🏁 Script executed:
# Search for IconButton patterns with aria-label to understand the codebase convention
rg -A 3 "IconButton" src/ | head -50Repository: kubescape/headlamp-plugin
Length of output: 3743
🏁 Script executed:
# Check if there's a specific accessibility pattern or utility for icon buttons
rg "aria-label" src/ --type tsx --type ts -B 2 -A 2 | head -60Repository: kubescape/headlamp-plugin
Length of output: 97
🏁 Script executed:
# Search for aria-label usage pattern across the codebase
rg "aria-label" src/ -B 2 -A 1 | head -80Repository: kubescape/headlamp-plugin
Length of output: 362
🏁 Script executed:
# Check if there are any accessible button patterns or utilities in the codebase
rg "IconButton" src/ -B 1 | grep -E "(aria-label|aria-)" | head -20Repository: kubescape/headlamp-plugin
Length of output: 285
🏁 Script executed:
# Check how Tooltip + IconButton patterns are currently handled in the codebase
rg -B 2 "Tooltip title=" src/vulnerabilities/ -A 3 | grep -E "(IconButton|aria-label|Tooltip)" | head -40Repository: kubescape/headlamp-plugin
Length of output: 1259
🏁 Script executed:
# Verify if there are any accessibility guidelines or config files in the repo
fd -E node_modules "(eslint|a11y|accessibility|a11y.*config)" . | head -20Repository: kubescape/headlamp-plugin
Length of output: 51
🏁 Script executed:
# Check the structure and imports to understand the framework being used
head -20 src/vulnerabilities/CVEResults.tsxRepository: kubescape/headlamp-plugin
Length of output: 773
Add an accessible label to the icon-only action button.
Line 133 renders an icon-only IconButton without an explicit accessible name. The Tooltip title provides visual feedback on hover but does not register as an accessible name for screen readers, making this action undiscoverable for assistive technology users.
Suggested fix
- <IconButton size="small" onClick={() => setExceptionFormWorkload(workload)}>
+ <IconButton
+ size="small"
+ aria-label="Create Security Exception"
+ onClick={() => setExceptionFormWorkload(workload)}
+ >
<Icon icon="mdi:shield-plus-outline" />
</IconButton>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Tooltip title="Create Security Exception"> | |
| <IconButton size="small" onClick={() => setExceptionFormWorkload(workload)}> | |
| <Icon icon="mdi:shield-plus-outline" /> | |
| </IconButton> | |
| <Tooltip title="Create Security Exception"> | |
| <IconButton | |
| size="small" | |
| aria-label="Create Security Exception" | |
| onClick={() => setExceptionFormWorkload(workload)} | |
| > | |
| <Icon icon="mdi:shield-plus-outline" /> | |
| </IconButton> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/vulnerabilities/CVEResults.tsx` around lines 132 - 135, The IconButton
inside the Tooltip (rendering Icon and calling setExceptionFormWorkload) lacks
an accessible name; add an explicit accessible label (for example
aria-label="Create Security Exception" or aria-labelledby pointing to a hidden
label) to the IconButton so screen readers can discover the action while keeping
the Tooltip for visual users.
Replaces the old ConfigMap-based exception system with the new kubescape.io/v1beta1 SecurityException and ClusterSecurityException CRDs. - New guided forms for posture and vulnerability exceptions at workload and cluster scope, with scope radio buttons, status/justification selects, and expiry support - Client-side exception application to compliance scan data (temporary until the operator handles it at scan time) - Per-CVE exception button in vulnerability workload detail findings table - Cluster-wide CVE exclusion from the CVEs tab - Cluster-wide control exclusion from the compliance controls list - Shared helpers (MetadataFields, VulnExceptionFields, sanitizeName, …) extracted to avoid duplication across form files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Matthijs Galesloot <mgalesloot@gmail.com>
mgalesloot
force-pushed
the
main
branch
2 times, most recently
from
718a454 to
5b01d9e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces the old ConfigMap-based exception system with the new kubescape.io/v1beta1 SecurityException and ClusterSecurityException CRDs.
Summary by CodeRabbit
Release Notes
New Features
Changes