EDM-3715 vulnerability management#636
Merged
celdrake merged 7 commits intoMay 15, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end vulnerability support: new OpenAPI types, API path routing and handlers, service enablement hooks, vulnerability list/summary/impact hooks, shared FlightCtlPageDrawer, Security Overview UI (card, summary, table, rows, details drawer, impact), device/fleet integration with keyed text filters (including CVE), table typing refactor, standalone/OCP routing, translations, and utility helpers. ChangesSecurity overview and vulnerabilities
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes |
celdrake
force-pushed
the
EDM-3715-vulnerability-management
branch
from
95d6323 to
026d26c
Compare
celdrake
commented
May 7, 2026
| font-weight: var(--pf-t--global--font--weight--body--bold); | ||
| } | ||
|
|
||
| .fctl-security-overview-summary-box.critical { |
Collaborator
Author
There was a problem hiding this comment.
The "severity" Patternfly tokens are not really integrated with other elements.
celdrake
force-pushed
the
EDM-3715-vulnerability-management
branch
3 times, most recently
from
47e8455 to
4597c67
Compare
celdrake
force-pushed
the
EDM-3715-vulnerability-management
branch
2 times, most recently
from
26b4900 to
272ed24
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
libs/ui-components/src/types/rbac.ts (1)
16-17: Confirm RBAC resource path before release.The "CELIA WIP" comment indicates the
vulnerabilitiesresource path needs backend RBAC confirmation. This coordination task should be tracked to ensure RBAC alignment before release.Would you like me to create a tracking issue for confirming the
vulnerabilitiesresource path with the backend RBAC implementation?🤖 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 `@libs/ui-components/src/types/rbac.ts` around lines 16 - 17, The WIP comment indicates the RBAC resource path for the enum member VULNERABILITY = 'vulnerabilities' needs backend confirmation; create a tracking issue in the repo/board to confirm the backend RBAC resource path, then replace the inline "CELIA-WIP" comment with a concise TODO that references the new issue ID (or link) and notes to verify/align the 'vulnerabilities' resource string with backend RBAC before release so the enum member (VULNERABILITY) can be finalized.libs/ui-components/src/hooks/useServicesEnabled.ts (1)
38-46: ⚡ Quick winDocument the error format contract to avoid silent breakage.
The error handling assumes
error.messageis a numeric string for status codes 500, 501, and 401—an implicit contract created by the apiCalls handlers in both apps/ocp-plugin and apps/standalone. This tight coupling works today because these specific status codes are intentionally thrown as pure numeric strings (e.g.,throw new Error(\${response.status}`)) with a comment "Return the status code to correctly detect that alerts are disabled." However, this contract is undocumented inuseServicesEnabled.ts`.If the error format changes or other error paths are used (e.g., formatted messages like
"Error 500: Internal Server Error"),Number(errorMessage)silently returnsNaN, and the fallback toisEnabled = truemasks the issue. Adding explicit documentation or a helper function to validate and extract status codes would prevent future maintainers from inadvertently breaking this coupling.🤖 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 `@libs/ui-components/src/hooks/useServicesEnabled.ts` around lines 38 - 46, The error handling in useServicesEnabled assumes error.message contains a pure numeric status code; add explicit documentation and a small validator/helper to extract and validate numeric status codes instead of trusting Number(error.message). Update the block referencing disabledStatusCodes and setIsEnabled (and abortController.signal.aborted) to call the helper (e.g., parseStatusCodeFromError) which returns a number or null, and only compare against disabledStatusCodes when a valid number is returned; also add a short comment above useServicesEnabled explaining the expected error.message contract so future callers/handlers know to throw plain numeric status strings.libs/ui-components/src/components/Device/DevicesPage/DeviceNameOnlyToolbarFilter.tsx (1)
23-25: 💤 Low valueMinor: Redundant dependency in useEffect.
setTextFilteris already captured in the memoizeddebouncedSetNameOrAliascallback, so including it directly in this effect's dependency array is unnecessary. This doesn't cause bugs but adds slight noise.Suggested cleanup
React.useEffect(() => { debouncedSetNameOrAlias(typingText); - }, [typingText, setTextFilter, debouncedSetNameOrAlias]); + }, [typingText, debouncedSetNameOrAlias]);🤖 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 `@libs/ui-components/src/components/Device/DevicesPage/DeviceNameOnlyToolbarFilter.tsx` around lines 23 - 25, The useEffect callback currently lists an unnecessary dependency; remove setTextFilter from the dependency array of the React.useEffect that calls debouncedSetNameOrAlias so the effect depends only on typingText and debouncedSetNameOrAlias (debouncedSetNameOrAlias already closes over setTextFilter), e.g., update the React.useEffect containing debouncedSetNameOrAlias(typingText) to only include typingText and debouncedSetNameOrAlias.libs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsx (2)
180-180: ⚡ Quick winInconsistent query parameter key: hardcoded
cveIdinstead ofFilterSearchParams.CveId.Similar to above, use the constant for consistency with the rest of the codebase.
♻️ Suggested fix
- <Link to={ROUTE.DEVICES} query={new URLSearchParams({ cveId }).toString()}> + <Link to={ROUTE.DEVICES} query={new URLSearchParams({ [FilterSearchParams.CveId]: cveId }).toString()}>🤖 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 `@libs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsx` at line 180, The Link to ROUTE.DEVICES is building the query with a hardcoded key "cveId" — change it to use the shared constant FilterSearchParams.CveId for consistency; update the URLSearchParams construction in the Link (the element using ROUTE.DEVICES) to pass new URLSearchParams({ [FilterSearchParams.CveId]: cveId }).toString() so the query key matches the rest of the codebase.
253-253: ⚡ Quick winInconsistent query parameter naming:
fleetIdvsFilterSearchParams.Fleet.The link uses hardcoded
fleetIdin the query string, but elsewhereFilterSearchParams.Fleetis used (which equals'fleetId'). For consistency and maintainability, use the constant.♻️ Suggested fix
- <Link to={ROUTE.DEVICES} query={new URLSearchParams({ cveId, fleetId: fleetName }).toString()}> + <Link to={ROUTE.DEVICES} query={new URLSearchParams({ [FilterSearchParams.CveId]: cveId, [FilterSearchParams.Fleet]: fleetName }).toString()}>🤖 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 `@libs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsx` at line 253, Link query uses hardcoded "fleetId" instead of the shared constant; update the Link in VulnerabilityImpact (the <Link to={ROUTE.DEVICES} ...> call) to build the query using FilterSearchParams.Fleet (which equals 'fleetId') instead of the literal string so naming is consistent and maintainable; replace new URLSearchParams({ cveId, fleetId: fleetName }).toString() with a construction that uses FilterSearchParams.Fleet as the key for fleetName.libs/ui-components/src/components/SecurityOverview/VulnerabilityDetailsDrawer.tsx (1)
77-85: 💤 Low valuePotential runtime error when accessing
findings[0]on an empty array.If
vulnerability.findingsis an empty array, accessingvulnerability.findings[0]will returnundefined, causingadvisoryIdanddescriptionto silently becomeundefined. This may be intentional, but if theVulnerabilityGrouptype guarantees at least one finding, consider adding a runtime guard or documenting this assumption.🤖 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 `@libs/ui-components/src/components/SecurityOverview/VulnerabilityDetailsDrawer.tsx` around lines 77 - 85, The branch in VulnerabilityDetailsDrawer.tsx that runs when isGrouped reads vulnerability.findings[0].advisoryId and .description without guarding for an empty findings array; add a runtime guard so advisoryId and description are only read when vulnerability.findings?.length > 0 (or fallback to a sensible default like vulnerability.advisoryId/description or an empty string) and ensure publishedAt still uses vulnerability.maxPublishedAt for grouped items; alternatively, assert/document that the VulnerabilityGroup type always contains at least one finding and add a runtime check that throws or logs if findings is empty to make the assumption explicit.libs/ui-components/src/components/Device/DevicesPage/DeviceToolbarFilters.tsx (1)
333-352: 💤 Low valueDebounced function may not cancel properly on dependency changes.
When
setTextFilterortchanges, a new debounced function is created, but the old one's pending invocation isn't cancelled. The cleanup in the useEffect (line 363) only cancels the current debounced function, not the previous one.This is unlikely to cause visible issues in practice since
setTextFilteris typically stable (from useCallback), but consider usinguseReffor the debounce function or ensuring the debounce is cancelled when dependencies change.🤖 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 `@libs/ui-components/src/components/Device/DevicesPage/DeviceToolbarFilters.tsx` around lines 333 - 352, The debouncedSetTextFilter created with debounce(...) can leave pending invocations when its dependencies (setTextFilter or t) change; replace the useMemo pattern with a ref-stored debounced function so you can cancel the previous instance when recreating: create a React.useRef for the debounced function (debouncedSetTextFilterRef), initialize or recreate it inside a useEffect that depends on [setTextFilter, t], and call debouncedSetTextFilterRef.current(...) where needed; ensure the effect cancels the previous debounced instance (call .cancel()) in its cleanup to avoid stray invocations and also cancel on unmount. Use the existing identifiers debouncedSetTextFilter (or replace with debouncedSetTextFilterRef), debounce, setTextFilter, and the CVE validation logic unchanged.libs/ui-components/src/components/Device/DevicesPage/useDevices.ts (1)
126-136: ⚡ Quick winRemove unused
nameOrAliasparameter fromuseDevicessignature.The
nameOrAliasparameter is not passed by any callers. The function already derives it fromtextFilters(line 44), making the parameter redundant.♻️ Suggested fix
export const useDevices = (args: { textFilters?: Partial<Record<DeviceTextFilterKey, string>>; - nameOrAlias?: string; ownerFleets?: string[]; activeStatuses?: FilterStatusMap; labels?: FlightCtlLabel[]; onlyDecommissioned: boolean; nextContinue?: string; onlyFleetless?: boolean; onPageFetched?: (data: DeviceList) => void; }): DevicesResult => {🤖 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 `@libs/ui-components/src/components/Device/DevicesPage/useDevices.ts` around lines 126 - 136, Remove the redundant nameOrAlias parameter from the useDevices function signature and related type for its args: delete the nameOrAlias?: string entry from the args object in useDevices and remove any code that references that parameter inside the function (the hook already reads the value from textFilters using DeviceTextFilterKey). Ensure the exported useDevices signature and any local references only rely on textFilters (and other existing keys), and run TypeScript to confirm no callers require nameOrAlias (update any tests or types that referenced the args shape if necessary).libs/ui-components/src/components/SecurityOverview/VulnerabilitiesTable.tsx (1)
4-5: ⚡ Quick winUse indexed-access typing with
import typefor maintainability.The current namespace-style access (
Vulnerability.severity) couples this component to the generated model's namespace structure. Switching to indexed-access typing with type-only imports removes this dependency and clarifies type-only intent.Proposed change
-import { Vulnerability, VulnerabilityGroup, VulnerabilityGroupList, VulnerabilityList } from '@flightctl/types/alpha'; +import type { Vulnerability, VulnerabilityGroup, VulnerabilityGroupList, VulnerabilityList } from '@flightctl/types/alpha'; -type VulnerabilitySeverity = Vulnerability.severity; +type VulnerabilitySeverity = NonNullable<Vulnerability['severity']>;🤖 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 `@libs/ui-components/src/components/SecurityOverview/VulnerabilitiesTable.tsx` around lines 4 - 5, Change the runtime namespace-style imports to type-only imports and use indexed-access types where the code references model members: replace the current import with an import type of Vulnerability, VulnerabilityGroup, VulnerabilityGroupList, VulnerabilityList from '@flightctl/types/alpha', and update any uses like Vulnerability.severity (or other property-access on the model types) to the indexed-access form Vulnerability['severity'] (and similarly for other properties) so the component depends only on types rather than the generated namespace at runtime.
🤖 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 `@libs/ui-components/src/components/common/FlightCtlPageDrawer.tsx`:
- Around line 13-24: getPageContentTop() reads document and is invoked eagerly
by usePageContentTop's useState initializer, causing SSR crashes; change the
initialization so no DOM access happens during render (either make
getPageContentTop return a safe default when typeof document === 'undefined' or
initialize topOffset with a constant like 60 and then call getPageContentTop()
inside a useEffect to setTopOffset). Update usePageContentTop (and keep
getPageContentTop) so DOM querying only runs inside useEffect or when document
is present, referencing the existing getPageContentTop and setTopOffset symbols
to locate and apply the fix.
In `@libs/ui-components/src/components/SecurityOverview/SecurityOverviewPage.tsx`:
- Around line 72-75: The UI currently shows the "not enabled" Alert while
useVulnerabilitiesEnabled() is still resolving; update the render guard in
SecurityOverviewPage to check isLoading first (e.g., only show the Alert when
isLoading is false and vulnerabilitiesEnabled is false) by changing the
condition that references vulnerabilitiesEnabled to include isLoading (for
example: if (!isLoading && !vulnerabilitiesEnabled) ...) or otherwise render a
loading state while isLoading is true so the transient incorrect Alert cannot
appear.
- Around line 36-40: The empty-state branch currently returns
SecurityOverviewEmptyState when itemCount === 0 regardless of request errors,
which masks backend failures; update the condition to only return that empty
state when there is no error (add a !error guard) so keep the existing check
(itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) but require
!error as well; locate the conditional around SecurityOverviewEmptyState in
SecurityOverviewPage.tsx and add the error check so ListPageBody can render
errors when present.
In `@libs/ui-components/src/hooks/useVulnerabilitySummary.ts`:
- Around line 13-47: The hook useVulnerabilitySummary currently swallows fetch
failures and always returns fallback counts; update
UseVulnerabilitySummaryResult to include an error?: Error | null field and
modify useVulnerabilitySummary to track an error state (e.g., const [error,
setError] = useState<Error | null>(null)), set error to null before loading,
setError(err) in the catch when the request fails (but do not set for aborts),
and return error alongside counts and isLoading so consumers can distinguish API
failures from legitimately empty results; ensure the abort handling still
prevents setting error after abort and preserve the existing isLoading
semantics.
In `@libs/ui-components/src/utils/dates.ts`:
- Around line 30-36: The getDateNoTimeDisplay function currently calls
dateFormatter().format(new Date(timestamp)) without validating the parsed date,
which throws a RangeError for malformed timestamps; update getDateNoTimeDisplay
to parse the timestamp into a Date object, check its validity (e.g., const d =
new Date(timestamp); if (isNaN(d.getTime())) return 'N/A';), and only then call
dateFormatter().format(d) so invalid or missing timestamps both return 'N/A'
(references: getDateNoTimeDisplay and dateFormatter).
---
Nitpick comments:
In
`@libs/ui-components/src/components/Device/DevicesPage/DeviceNameOnlyToolbarFilter.tsx`:
- Around line 23-25: The useEffect callback currently lists an unnecessary
dependency; remove setTextFilter from the dependency array of the
React.useEffect that calls debouncedSetNameOrAlias so the effect depends only on
typingText and debouncedSetNameOrAlias (debouncedSetNameOrAlias already closes
over setTextFilter), e.g., update the React.useEffect containing
debouncedSetNameOrAlias(typingText) to only include typingText and
debouncedSetNameOrAlias.
In
`@libs/ui-components/src/components/Device/DevicesPage/DeviceToolbarFilters.tsx`:
- Around line 333-352: The debouncedSetTextFilter created with debounce(...) can
leave pending invocations when its dependencies (setTextFilter or t) change;
replace the useMemo pattern with a ref-stored debounced function so you can
cancel the previous instance when recreating: create a React.useRef for the
debounced function (debouncedSetTextFilterRef), initialize or recreate it inside
a useEffect that depends on [setTextFilter, t], and call
debouncedSetTextFilterRef.current(...) where needed; ensure the effect cancels
the previous debounced instance (call .cancel()) in its cleanup to avoid stray
invocations and also cancel on unmount. Use the existing identifiers
debouncedSetTextFilter (or replace with debouncedSetTextFilterRef), debounce,
setTextFilter, and the CVE validation logic unchanged.
In `@libs/ui-components/src/components/Device/DevicesPage/useDevices.ts`:
- Around line 126-136: Remove the redundant nameOrAlias parameter from the
useDevices function signature and related type for its args: delete the
nameOrAlias?: string entry from the args object in useDevices and remove any
code that references that parameter inside the function (the hook already reads
the value from textFilters using DeviceTextFilterKey). Ensure the exported
useDevices signature and any local references only rely on textFilters (and
other existing keys), and run TypeScript to confirm no callers require
nameOrAlias (update any tests or types that referenced the args shape if
necessary).
In `@libs/ui-components/src/components/SecurityOverview/VulnerabilitiesTable.tsx`:
- Around line 4-5: Change the runtime namespace-style imports to type-only
imports and use indexed-access types where the code references model members:
replace the current import with an import type of Vulnerability,
VulnerabilityGroup, VulnerabilityGroupList, VulnerabilityList from
'@flightctl/types/alpha', and update any uses like Vulnerability.severity (or
other property-access on the model types) to the indexed-access form
Vulnerability['severity'] (and similarly for other properties) so the component
depends only on types rather than the generated namespace at runtime.
In
`@libs/ui-components/src/components/SecurityOverview/VulnerabilityDetailsDrawer.tsx`:
- Around line 77-85: The branch in VulnerabilityDetailsDrawer.tsx that runs when
isGrouped reads vulnerability.findings[0].advisoryId and .description without
guarding for an empty findings array; add a runtime guard so advisoryId and
description are only read when vulnerability.findings?.length > 0 (or fallback
to a sensible default like vulnerability.advisoryId/description or an empty
string) and ensure publishedAt still uses vulnerability.maxPublishedAt for
grouped items; alternatively, assert/document that the VulnerabilityGroup type
always contains at least one finding and add a runtime check that throws or logs
if findings is empty to make the assumption explicit.
In `@libs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsx`:
- Line 180: The Link to ROUTE.DEVICES is building the query with a hardcoded key
"cveId" — change it to use the shared constant FilterSearchParams.CveId for
consistency; update the URLSearchParams construction in the Link (the element
using ROUTE.DEVICES) to pass new URLSearchParams({ [FilterSearchParams.CveId]:
cveId }).toString() so the query key matches the rest of the codebase.
- Line 253: Link query uses hardcoded "fleetId" instead of the shared constant;
update the Link in VulnerabilityImpact (the <Link to={ROUTE.DEVICES} ...> call)
to build the query using FilterSearchParams.Fleet (which equals 'fleetId')
instead of the literal string so naming is consistent and maintainable; replace
new URLSearchParams({ cveId, fleetId: fleetName }).toString() with a
construction that uses FilterSearchParams.Fleet as the key for fleetName.
In `@libs/ui-components/src/hooks/useServicesEnabled.ts`:
- Around line 38-46: The error handling in useServicesEnabled assumes
error.message contains a pure numeric status code; add explicit documentation
and a small validator/helper to extract and validate numeric status codes
instead of trusting Number(error.message). Update the block referencing
disabledStatusCodes and setIsEnabled (and abortController.signal.aborted) to
call the helper (e.g., parseStatusCodeFromError) which returns a number or null,
and only compare against disabledStatusCodes when a valid number is returned;
also add a short comment above useServicesEnabled explaining the expected
error.message contract so future callers/handlers know to throw plain numeric
status strings.
In `@libs/ui-components/src/types/rbac.ts`:
- Around line 16-17: The WIP comment indicates the RBAC resource path for the
enum member VULNERABILITY = 'vulnerabilities' needs backend confirmation; create
a tracking issue in the repo/board to confirm the backend RBAC resource path,
then replace the inline "CELIA-WIP" comment with a concise TODO that references
the new issue ID (or link) and notes to verify/align the 'vulnerabilities'
resource string with backend RBAC before release so the enum member
(VULNERABILITY) can be finalized.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 14ca6724-36a8-4fd0-b442-a8a1ac0f958f
📒 Files selected for processing (85)
apps/ocp-plugin/console-extensions.jsonapps/ocp-plugin/package.jsonapps/ocp-plugin/src/components/AppContext/AppContext.tsxapps/ocp-plugin/src/components/SecurityOverview/SecurityOverviewPage.tsxapps/ocp-plugin/src/utils/apiCalls.tsapps/standalone/src/app/routes.tsxapps/standalone/src/app/utils/apiCalls.tseslint.config.jslibs/i18n/locales/en/translation.jsonlibs/types/alpha/index.tslibs/types/alpha/models/AffectedFleet.tslibs/types/alpha/models/CatalogItemVersion.tslibs/types/alpha/models/CveCountsBySeverity.tslibs/types/alpha/models/DeviceCountsBySeverity.tslibs/types/alpha/models/DeviceVulnerabilitySummaryResponse.tslibs/types/alpha/models/FleetVulnerabilitySummary.tslibs/types/alpha/models/FleetVulnerabilitySummaryResponse.tslibs/types/alpha/models/SemVer.tslibs/types/alpha/models/SemVerRange.tslibs/types/alpha/models/Vulnerability.tslibs/types/alpha/models/VulnerabilityGroup.tslibs/types/alpha/models/VulnerabilityGroupItem.tslibs/types/alpha/models/VulnerabilityGroupList.tslibs/types/alpha/models/VulnerabilityImageRef.tslibs/types/alpha/models/VulnerabilityImpact.tslibs/types/alpha/models/VulnerabilityList.tslibs/types/alpha/models/VulnerabilitySeveritySummary.tslibs/types/alpha/models/VulnerabilitySummaryResponse.tslibs/types/imagebuilder/models/ImageBuildConditionReason.tslibs/types/models/K8sProviderSpec.tslibs/ui-components/src/components/AuthProvider/AuthProvidersPage.tsxlibs/ui-components/src/components/Catalog/CatalogItemDetails.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SelectTargetStep.tsxlibs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsxlibs/ui-components/src/components/Device/DeviceDetails/DeviceVulnerabilities.tsxlibs/ui-components/src/components/Device/DevicesPage/DecommissionedDevicesTable.tsxlibs/ui-components/src/components/Device/DevicesPage/DeviceNameOnlyToolbarFilter.tsxlibs/ui-components/src/components/Device/DevicesPage/DeviceTableToolbar.tsxlibs/ui-components/src/components/Device/DevicesPage/DeviceToolbarFilters.tsxlibs/ui-components/src/components/Device/DevicesPage/DevicesPage.tsxlibs/ui-components/src/components/Device/DevicesPage/EnrolledDeviceTableRow.tsxlibs/ui-components/src/components/Device/DevicesPage/EnrolledDevicesTable.tsxlibs/ui-components/src/components/Device/DevicesPage/useDeviceBackendFilters.tslibs/ui-components/src/components/Device/DevicesPage/useDevices.tslibs/ui-components/src/components/EnrollmentRequest/EnrollmentRequestList.tsxlibs/ui-components/src/components/Fleet/FleetDetails/FleetDetailsContent.tsxlibs/ui-components/src/components/Fleet/FleetDetails/FleetVulnerabilities.tsxlibs/ui-components/src/components/Fleet/FleetsPage.tsxlibs/ui-components/src/components/ImageBuilds/ImageBuildAndExportStatus.tsxlibs/ui-components/src/components/ImageBuilds/ImageBuildsPage.tsxlibs/ui-components/src/components/OverviewPage/Cards/SecurityOverview/SecurityOverviewCard.tsxlibs/ui-components/src/components/OverviewPage/Cards/Tasks/TasksCard.tsxlibs/ui-components/src/components/OverviewPage/Overview.tsxlibs/ui-components/src/components/OverviewPage/OverviewPage.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsxlibs/ui-components/src/components/SecurityOverview/SecurityOverviewPage.tsxlibs/ui-components/src/components/SecurityOverview/SecurityOverviewSummary.csslibs/ui-components/src/components/SecurityOverview/SecurityOverviewSummary.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilitiesEmptyState.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilitiesTable.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilitiesTableRow.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityAffectedImages.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityDescription.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityDetailsDrawer.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsxlibs/ui-components/src/components/Status/VulnerabilitySeverityStatus.tsxlibs/ui-components/src/components/Table/Table.tsxlibs/ui-components/src/components/common/FlightCtlPageDrawer.tsxlibs/ui-components/src/constants.tslibs/ui-components/src/hooks/useAffectedImagesExpand.tslibs/ui-components/src/hooks/useAlertsEnabled.tslibs/ui-components/src/hooks/useAppContext.tsxlibs/ui-components/src/hooks/useNavigate.tsxlibs/ui-components/src/hooks/useServicesEnabled.tslibs/ui-components/src/hooks/useVulnerabilities.tslibs/ui-components/src/hooks/useVulnerabilityImpact.tslibs/ui-components/src/hooks/useVulnerabilitySummary.tslibs/ui-components/src/types/rbac.tslibs/ui-components/src/utils/api.tslibs/ui-components/src/utils/dates.tslibs/ui-components/src/utils/imageBuilds.tslibs/ui-components/src/utils/status/devices.tslibs/ui-components/src/utils/status/vulnerabilities.tslibs/ui-components/src/utils/vulnerabilities.ts
💤 Files with no reviewable changes (1)
- libs/ui-components/src/hooks/useAlertsEnabled.ts
Comment on lines
+36
to
+40
| if (itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) { | ||
| // The link to this page is only shown when some vulnerabilities are present. | ||
| // If the user navigates to it directly, we show the empty state assuming there are some devices | ||
| return <SecurityOverviewEmptyState hasDevices />; | ||
| } |
There was a problem hiding this comment.
Don’t mask backend failures with the empty state.
When itemCount === 0, this branch returns before ListPageBody can render error, so request failures can appear as a normal empty state. Add an !error guard here.
Suggested patch
- if (itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) {
+ if (!error && itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) {
// The link to this page is only shown when some vulnerabilities are present.
// If the user navigates to it directly, we show the empty state assuming there are some devices
return <SecurityOverviewEmptyState hasDevices />;
}📝 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
| if (itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) { | |
| // The link to this page is only shown when some vulnerabilities are present. | |
| // If the user navigates to it directly, we show the empty state assuming there are some devices | |
| return <SecurityOverviewEmptyState hasDevices />; | |
| } | |
| if (!error && itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) { | |
| // The link to this page is only shown when some vulnerabilities are present. | |
| // If the user navigates to it directly, we show the empty state assuming there are some devices | |
| return <SecurityOverviewEmptyState hasDevices />; | |
| } |
🤖 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 `@libs/ui-components/src/components/SecurityOverview/SecurityOverviewPage.tsx`
around lines 36 - 40, The empty-state branch currently returns
SecurityOverviewEmptyState when itemCount === 0 regardless of request errors,
which masks backend failures; update the condition to only return that empty
state when there is no error (add a !error guard) so keep the existing check
(itemCount === 0 && !hasFiltersEnabled && !isLoading && !isUpdating) but require
!error as well; locate the conditional around SecurityOverviewEmptyState in
SecurityOverviewPage.tsx and add the error check so ListPageBody can render
errors when present.
Comment on lines
+72
to
+75
| const [vulnerabilitiesEnabled, canListVulnerabilities, isLoading] = useVulnerabilitiesEnabled(); | ||
| if (!vulnerabilitiesEnabled) { | ||
| return <Alert isInline variant="info" title={t('Vulnerability reporting is not enabled in this environment.')} />; | ||
| } |
There was a problem hiding this comment.
Check loading before rendering the “not enabled” alert.
This branch can show a misleading disabled message while useVulnerabilitiesEnabled() is still resolving. Gate on isLoading first (or include it in the condition) to avoid transient incorrect UI state.
🤖 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 `@libs/ui-components/src/components/SecurityOverview/SecurityOverviewPage.tsx`
around lines 72 - 75, The UI currently shows the "not enabled" Alert while
useVulnerabilitiesEnabled() is still resolving; update the render guard in
SecurityOverviewPage to check isLoading first (e.g., only show the Alert when
isLoading is false and vulnerabilitiesEnabled is false) by changing the
condition that references vulnerabilitiesEnabled to include isLoading (for
example: if (!isLoading && !vulnerabilitiesEnabled) ...) or otherwise render a
loading state while isLoading is true so the transient incorrect Alert cannot
appear.
Comment on lines
+13
to
+47
| export const useVulnerabilitySummary = (): UseVulnerabilitySummaryResult => { | ||
| const { get } = useFetch(); | ||
| const [counts, setCounts] = React.useState<CveCountsBySeverity | null>(null); | ||
| const isLoading = counts === null; | ||
|
|
||
| React.useEffect(() => { | ||
| const abortController = new AbortController(); | ||
|
|
||
| const load = async () => { | ||
| setCounts(null); | ||
| try { | ||
| const summaryRes = await get<VulnerabilitySummaryResponse>('vulnerabilities/summary', abortController.signal); | ||
| if (abortController.signal.aborted) { | ||
| return; | ||
| } | ||
| setCounts(summaryRes.cvesBySeverity); | ||
| } catch { | ||
| if (!abortController.signal.aborted) { | ||
| setCounts(emptySeverityCounts); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| void load(); | ||
|
|
||
| return () => { | ||
| abortController.abort(); | ||
| }; | ||
| }, [get]); | ||
|
|
||
| return { | ||
| counts: counts ?? emptySeverityCounts, | ||
| isLoading, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Missing error state exposure could mislead users.
The hook silently falls back to zero counts on fetch failure without exposing an error state. Consuming components cannot distinguish between "no vulnerabilities" and "API failure", which could display misleading security information.
Impact:
- Security dashboards show "0 vulnerabilities" when the API is unavailable
- Users have no visibility into data freshness or availability
- Monitoring and alerting cannot detect fetch failures
Recommendation:
Add an error field to UseVulnerabilitySummaryResult and expose fetch failures to the consuming component.
🔧 Proposed fix to expose error state
export type UseVulnerabilitySummaryResult = {
counts: CveCountsBySeverity;
isLoading: boolean;
+ error: Error | null;
};
const emptySeverityCounts = { total: 0, critical: 0, high: 0, medium: 0, low: 0, none: 0, unknown: 0 };
export const useVulnerabilitySummary = (): UseVulnerabilitySummaryResult => {
const { get } = useFetch();
const [counts, setCounts] = React.useState<CveCountsBySeverity | null>(null);
+ const [error, setError] = React.useState<Error | null>(null);
const isLoading = counts === null;
React.useEffect(() => {
const abortController = new AbortController();
const load = async () => {
setCounts(null);
+ setError(null);
try {
const summaryRes = await get<VulnerabilitySummaryResponse>('vulnerabilities/summary', abortController.signal);
if (abortController.signal.aborted) {
return;
}
setCounts(summaryRes.cvesBySeverity);
- } catch {
+ } catch (e) {
if (!abortController.signal.aborted) {
- setCounts(emptySeverityCounts);
+ setError(e instanceof Error ? e : new Error('Failed to fetch vulnerability summary'));
+ setCounts(emptySeverityCounts);
}
}
};
void load();
return () => {
abortController.abort();
};
}, [get]);
return {
counts: counts ?? emptySeverityCounts,
isLoading,
+ error,
};
};The consuming component can then show an error state when error is not null.
🤖 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 `@libs/ui-components/src/hooks/useVulnerabilitySummary.ts` around lines 13 -
47, The hook useVulnerabilitySummary currently swallows fetch failures and
always returns fallback counts; update UseVulnerabilitySummaryResult to include
an error?: Error | null field and modify useVulnerabilitySummary to track an
error state (e.g., const [error, setError] = useState<Error | null>(null)), set
error to null before loading, setError(err) in the catch when the request fails
(but do not set for aborts), and return error alongside counts and isLoading so
consumers can distinguish API failures from legitimately empty results; ensure
the abort handling still prevents setting error after abort and preserve the
existing isLoading semantics.
celdrake
commented
May 8, 2026
celdrake
force-pushed
the
EDM-3715-vulnerability-management
branch
from
943ad2f to
dc07a77
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/types/models/EventDetails.ts (1)
8-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
DeviceVulnerabilityCveDetailstoEventDetailsunionLine 8 imports
DeviceVulnerabilityCveDetails, but Line 23 omits it fromEventDetails. This leaves vulnerability-CVE events outside the declared event contract and can break typed consumers handling event details.Proposed fix
-export type EventDetails = (ResourceUpdatedDetails | DeviceOwnershipChangedDetails | DeviceMultipleOwnersDetectedDetails | DeviceMultipleOwnersResolvedDetails | InternalTaskFailedDetails | InternalTaskPermanentlyFailedDetails | ResourceSyncCompletedDetails | ReferencedRepositoryUpdatedDetails | FleetRolloutStartedDetails | FleetRolloutFailedDetails | FleetRolloutCompletedDetails | FleetRolloutBatchDispatchedDetails | FleetRolloutBatchCompletedDetails | FleetRolloutDeviceSelectedDetails); +export type EventDetails = (ResourceUpdatedDetails | DeviceOwnershipChangedDetails | DeviceMultipleOwnersDetectedDetails | DeviceMultipleOwnersResolvedDetails | InternalTaskFailedDetails | InternalTaskPermanentlyFailedDetails | ResourceSyncCompletedDetails | ReferencedRepositoryUpdatedDetails | FleetRolloutStartedDetails | FleetRolloutFailedDetails | FleetRolloutCompletedDetails | FleetRolloutBatchDispatchedDetails | FleetRolloutBatchCompletedDetails | FleetRolloutDeviceSelectedDetails | DeviceVulnerabilityCveDetails);As per coding guidelines, "Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity."
🤖 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 `@libs/types/models/EventDetails.ts` around lines 8 - 23, The EventDetails union type is missing DeviceVulnerabilityCveDetails even though it is imported; update the exported EventDetails union to include DeviceVulnerabilityCveDetails alongside the other union members (i.e., add DeviceVulnerabilityCveDetails into the union expression in EventDetails), then run the TypeScript type check/build and adjust any affected consumers or tests to account for the newly supported event detail type.
🤖 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 `@apps/standalone/src/app/utils/apiCalls.ts`:
- Around line 126-142: handleVulnerabilityJSONResponse lacks explicit 401
handling; update the function (handleVulnerabilityJSONResponse) to mirror
handleApiJSONResponse by checking if response.status === 401, call
redirectToLogin() and then throw an appropriate Error (e.g., using
getErrorMsgFromApiResponse(response) or a short message) so unauthorized
vulnerability responses redirect to login instead of producing a generic error.
In
`@libs/ui-components/src/components/Device/DevicesPage/useDeviceBackendFilters.ts`:
- Around line 118-125: paramsRef is initialized once and never kept in sync with
external searchParams, so setters like setOnlyFleetless build new params from
stale paramsRef.current; add an effect that updates paramsRef.current whenever
searchParams changes (e.g., React.useEffect(() => { paramsRef.current =
searchParams }, [searchParams])) so getNewParams called in setOnlyFleetless and
other callbacks uses the latest params, and ensure any callbacks that rely on
paramsRef remain stable (keep updateSearchParams in their deps) while reading
the up-to-date paramsRef.current.
In `@libs/ui-components/src/components/OverviewPage/Overview.tsx`:
- Around line 21-30: Compute a single flag showSecurityOverview =
vulnerabilitiesEnabled && canListVulnerabilities (using the values returned from
useVulnerabilitiesEnabled and checkPermissions) and use that flag everywhere:
set vulnColumns = showSecurityOverview ? 6 : 12, include showSecurityOverview in
the PageWithPermissions.allowed expression instead of relying on
vulnerabilitiesEnabled alone, include vulnerabilitiesLoading only when
showSecurityOverview is true in PageWithPermissions.loading, and conditionally
render SecurityOverviewCard based on showSecurityOverview so it never mounts for
users without RBAC.
In
`@libs/ui-components/src/components/SecurityOverview/VulnerabilityDetailsDrawer.tsx`:
- Around line 46-57: In VulnerabilityDetailsDrawer, the grouped-CVE branch
currently uses vulnerability.findings[0] as the reference source; change this to
locate the first finding in vulnerability.findings that contains reference
metadata (e.g., a non-empty link or advisory), then set publishedAt =
vulnerability.maxPublishedAt and set description and hasReferences based on that
selected finding (not always findings[0]); persist that selectedFinding
reference so VulnerabilityReferences renders using the same finding. Ensure you
update the isGrouped branch where description, hasReferences and any place that
passes a finding to VulnerabilityReferences to use this first-with-link
selection.
---
Outside diff comments:
In `@libs/types/models/EventDetails.ts`:
- Around line 8-23: The EventDetails union type is missing
DeviceVulnerabilityCveDetails even though it is imported; update the exported
EventDetails union to include DeviceVulnerabilityCveDetails alongside the other
union members (i.e., add DeviceVulnerabilityCveDetails into the union expression
in EventDetails), then run the TypeScript type check/build and adjust any
affected consumers or tests to account for the newly supported event detail
type.
🪄 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: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: d75308c7-666b-4329-ba94-c3e31cf5f7c9
📒 Files selected for processing (92)
apps/ocp-plugin/console-extensions.jsonapps/ocp-plugin/package.jsonapps/ocp-plugin/src/components/AppContext/AppContext.tsxapps/ocp-plugin/src/components/SecurityOverview/SecurityOverviewPage.tsxapps/ocp-plugin/src/utils/apiCalls.tsapps/standalone/src/app/routes.tsxapps/standalone/src/app/utils/apiCalls.tseslint.config.jslibs/i18n/locales/en/translation.jsonlibs/types/alpha/index.tslibs/types/alpha/models/AffectedFleet.tslibs/types/alpha/models/CatalogItemVersion.tslibs/types/alpha/models/CveCountsBySeverity.tslibs/types/alpha/models/DeviceCountsBySeverity.tslibs/types/alpha/models/DeviceVulnerabilitySummaryResponse.tslibs/types/alpha/models/FleetVulnerabilitySummary.tslibs/types/alpha/models/FleetVulnerabilitySummaryResponse.tslibs/types/alpha/models/SemVer.tslibs/types/alpha/models/SemVerRange.tslibs/types/alpha/models/Vulnerability.tslibs/types/alpha/models/VulnerabilityGroup.tslibs/types/alpha/models/VulnerabilityGroupItem.tslibs/types/alpha/models/VulnerabilityGroupList.tslibs/types/alpha/models/VulnerabilityImageRef.tslibs/types/alpha/models/VulnerabilityImpact.tslibs/types/alpha/models/VulnerabilityList.tslibs/types/alpha/models/VulnerabilitySeveritySummary.tslibs/types/alpha/models/VulnerabilitySummaryResponse.tslibs/types/imagebuilder/models/ImageBuildConditionReason.tslibs/types/index.tslibs/types/models/DependenciesSync.tslibs/types/models/DeviceSpec.tslibs/types/models/DeviceVulnerabilityCveDetails.tslibs/types/models/EventDetails.tslibs/types/models/FleetSpec.tslibs/types/models/K8sProviderSpec.tslibs/ui-components/src/components/AuthProvider/AuthProvidersPage.tsxlibs/ui-components/src/components/Catalog/CatalogItemDetails.tsxlibs/ui-components/src/components/Catalog/InstallWizard/steps/SelectTargetStep.tsxlibs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsxlibs/ui-components/src/components/Device/DeviceDetails/DeviceVulnerabilities.tsxlibs/ui-components/src/components/Device/DevicesPage/DecommissionedDevicesTable.tsxlibs/ui-components/src/components/Device/DevicesPage/DeviceNameOnlyToolbarFilter.tsxlibs/ui-components/src/components/Device/DevicesPage/DeviceTableToolbar.tsxlibs/ui-components/src/components/Device/DevicesPage/DeviceToolbarFilters.tsxlibs/ui-components/src/components/Device/DevicesPage/DevicesPage.tsxlibs/ui-components/src/components/Device/DevicesPage/EnrolledDeviceTableRow.tsxlibs/ui-components/src/components/Device/DevicesPage/EnrolledDevicesTable.tsxlibs/ui-components/src/components/Device/DevicesPage/useDeviceBackendFilters.tslibs/ui-components/src/components/Device/DevicesPage/useDevices.tslibs/ui-components/src/components/EnrollmentRequest/EnrollmentRequestList.tsxlibs/ui-components/src/components/Fleet/FleetDetails/FleetDetailsContent.tsxlibs/ui-components/src/components/Fleet/FleetDetails/FleetVulnerabilities.tsxlibs/ui-components/src/components/Fleet/FleetsPage.tsxlibs/ui-components/src/components/ImageBuilds/ImageBuildAndExportStatus.tsxlibs/ui-components/src/components/ImageBuilds/ImageBuildsPage.tsxlibs/ui-components/src/components/OverviewPage/Cards/SecurityOverview/SecurityOverviewCard.tsxlibs/ui-components/src/components/OverviewPage/Cards/Tasks/TasksCard.tsxlibs/ui-components/src/components/OverviewPage/Overview.tsxlibs/ui-components/src/components/OverviewPage/OverviewPage.tsxlibs/ui-components/src/components/Repository/RepositoryList.tsxlibs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsxlibs/ui-components/src/components/SecurityOverview/SecurityOverviewPage.tsxlibs/ui-components/src/components/SecurityOverview/SecurityOverviewSummary.csslibs/ui-components/src/components/SecurityOverview/SecurityOverviewSummary.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilitiesEmptyState.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilitiesTable.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilitiesTableRow.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityAffectedImages.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityDescription.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityDetailsDrawer.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsxlibs/ui-components/src/components/SecurityOverview/VulnerabilityReferences.tsxlibs/ui-components/src/components/Status/VulnerabilitySeverityStatus.tsxlibs/ui-components/src/components/Table/Table.tsxlibs/ui-components/src/components/common/FlightCtlPageDrawer.tsxlibs/ui-components/src/constants.tslibs/ui-components/src/hooks/useAffectedImagesExpand.tslibs/ui-components/src/hooks/useAlertsEnabled.tslibs/ui-components/src/hooks/useAppContext.tsxlibs/ui-components/src/hooks/useNavigate.tsxlibs/ui-components/src/hooks/useServicesEnabled.tslibs/ui-components/src/hooks/useVulnerabilities.tslibs/ui-components/src/hooks/useVulnerabilityImpact.tslibs/ui-components/src/hooks/useVulnerabilitySummary.tslibs/ui-components/src/types/rbac.tslibs/ui-components/src/utils/api.tslibs/ui-components/src/utils/dates.tslibs/ui-components/src/utils/imageBuilds.tslibs/ui-components/src/utils/status/devices.tslibs/ui-components/src/utils/status/vulnerabilities.tslibs/ui-components/src/utils/vulnerabilities.ts
💤 Files with no reviewable changes (1)
- libs/ui-components/src/hooks/useAlertsEnabled.ts
✅ Files skipped from review due to trivial changes (28)
- libs/types/models/DeviceSpec.ts
- libs/ui-components/src/components/OverviewPage/Cards/Tasks/TasksCard.tsx
- libs/types/alpha/models/SemVer.ts
- libs/types/models/DependenciesSync.ts
- libs/types/index.ts
- libs/ui-components/src/components/SecurityOverview/VulnerabilityDescription.tsx
- libs/types/alpha/models/SemVerRange.ts
- libs/types/alpha/models/VulnerabilityImageRef.ts
- libs/types/alpha/models/DeviceVulnerabilitySummaryResponse.ts
- libs/ui-components/src/components/ImageBuilds/ImageBuildsPage.tsx
- libs/types/alpha/models/AffectedFleet.ts
- libs/types/alpha/models/FleetVulnerabilitySummaryResponse.ts
- libs/types/models/DeviceVulnerabilityCveDetails.ts
- libs/ui-components/src/components/Device/DevicesPage/EnrolledDeviceTableRow.tsx
- libs/types/alpha/models/FleetVulnerabilitySummary.ts
- libs/types/alpha/models/VulnerabilitySummaryResponse.ts
- libs/types/alpha/models/VulnerabilityList.ts
- libs/types/alpha/models/VulnerabilitySeveritySummary.ts
- libs/types/models/K8sProviderSpec.ts
- apps/ocp-plugin/src/components/AppContext/AppContext.tsx
- libs/types/alpha/models/CveCountsBySeverity.ts
- libs/types/alpha/models/Vulnerability.ts
- libs/ui-components/src/hooks/useNavigate.tsx
- libs/ui-components/src/components/EnrollmentRequest/EnrollmentRequestList.tsx
- libs/types/alpha/models/VulnerabilityGroup.ts
- libs/types/alpha/models/VulnerabilityGroupList.ts
- libs/ui-components/src/utils/api.ts
- libs/types/alpha/index.ts
🚧 Files skipped from review as they are similar to previous changes (51)
- libs/ui-components/src/components/OverviewPage/OverviewPage.tsx
- libs/ui-components/src/components/Catalog/InstallWizard/steps/SelectTargetStep.tsx
- libs/types/alpha/models/VulnerabilityImpact.ts
- libs/types/alpha/models/VulnerabilityGroupItem.ts
- libs/ui-components/src/components/AuthProvider/AuthProvidersPage.tsx
- eslint.config.js
- libs/ui-components/src/components/Device/DeviceDetails/DeviceVulnerabilities.tsx
- libs/ui-components/src/components/Fleet/FleetsPage.tsx
- libs/ui-components/src/components/ResourceSync/RepositoryResourceSyncList.tsx
- libs/ui-components/src/components/Fleet/FleetDetails/FleetDetailsContent.tsx
- libs/ui-components/src/components/Repository/RepositoryList.tsx
- libs/ui-components/src/components/Fleet/FleetDetails/FleetVulnerabilities.tsx
- apps/ocp-plugin/package.json
- libs/ui-components/src/components/SecurityOverview/VulnerabilitiesEmptyState.tsx
- libs/ui-components/src/hooks/useAffectedImagesExpand.ts
- libs/types/alpha/models/CatalogItemVersion.ts
- libs/types/alpha/models/DeviceCountsBySeverity.ts
- libs/ui-components/src/components/Status/VulnerabilitySeverityStatus.tsx
- apps/ocp-plugin/src/components/SecurityOverview/SecurityOverviewPage.tsx
- libs/ui-components/src/utils/status/vulnerabilities.ts
- apps/standalone/src/app/routes.tsx
- libs/types/imagebuilder/models/ImageBuildConditionReason.ts
- libs/ui-components/src/components/SecurityOverview/SecurityOverviewPage.tsx
- libs/ui-components/src/components/ImageBuilds/ImageBuildAndExportStatus.tsx
- libs/ui-components/src/components/common/FlightCtlPageDrawer.tsx
- apps/ocp-plugin/console-extensions.json
- libs/ui-components/src/utils/dates.ts
- libs/ui-components/src/hooks/useVulnerabilitySummary.ts
- libs/ui-components/src/hooks/useServicesEnabled.ts
- libs/ui-components/src/components/SecurityOverview/VulnerabilitiesTableRow.tsx
- libs/ui-components/src/components/SecurityOverview/VulnerabilityAffectedImages.tsx
- libs/ui-components/src/components/Device/DevicesPage/DeviceToolbarFilters.tsx
- libs/ui-components/src/components/Device/DevicesPage/DevicesPage.tsx
- libs/ui-components/src/hooks/useVulnerabilityImpact.ts
- libs/ui-components/src/utils/status/devices.ts
- libs/ui-components/src/components/Device/DevicesPage/useDevices.ts
- libs/ui-components/src/components/Table/Table.tsx
- libs/ui-components/src/utils/vulnerabilities.ts
- libs/ui-components/src/types/rbac.ts
- libs/ui-components/src/components/Device/DevicesPage/DeviceNameOnlyToolbarFilter.tsx
- libs/ui-components/src/hooks/useVulnerabilities.ts
- libs/ui-components/src/utils/imageBuilds.ts
- libs/ui-components/src/components/SecurityOverview/VulnerabilitiesTable.tsx
- libs/ui-components/src/components/SecurityOverview/SecurityOverviewSummary.css
- libs/ui-components/src/components/SecurityOverview/SecurityOverviewSummary.tsx
- libs/ui-components/src/components/Catalog/CatalogItemDetails.tsx
- libs/ui-components/src/components/Device/DevicesPage/DecommissionedDevicesTable.tsx
- libs/ui-components/src/components/Device/DevicesPage/DeviceTableToolbar.tsx
- libs/ui-components/src/components/SecurityOverview/VulnerabilityImpact.tsx
- apps/ocp-plugin/src/utils/apiCalls.ts
- libs/ui-components/src/components/Device/DeviceDetails/DeviceDetailsTab.tsx
Comment on lines
+126
to
+142
| const handleVulnerabilityJSONResponse = async <R>(response: Response): Promise<R> => { | ||
| if (response.ok) { | ||
| const data = (await response.json()) as R; | ||
| return data; | ||
| } | ||
|
|
||
| if (response.status === 404) { | ||
| throw new Error(`Error ${response.status}: ${response.statusText}`); | ||
| } | ||
|
|
||
| // API returns 501 for disabled vulnerabilities API. | ||
| if (response.status === 501) { | ||
| throw new Error(`${response.status}`); | ||
| } | ||
|
|
||
| throw new Error(await getErrorMsgFromApiResponse(response)); | ||
| }; |
There was a problem hiding this comment.
Missing 401 handling for vulnerability responses.
Unlike handleApiJSONResponse (which calls redirectToLogin() on 401) and handleAlertsJSONResponse (which treats 401 as "disabled"), handleVulnerabilityJSONResponse has no explicit 401 handling. This means unauthorized requests to vulnerability endpoints will throw a generic error instead of redirecting users to login.
🔧 Proposed fix to add 401 handling
const handleVulnerabilityJSONResponse = async <R>(response: Response): Promise<R> => {
if (response.ok) {
const data = (await response.json()) as R;
return data;
}
if (response.status === 404) {
throw new Error(`Error ${response.status}: ${response.statusText}`);
}
+ if (response.status === 401) {
+ redirectToLogin();
+ }
+
// API returns 501 for disabled vulnerabilities API.
if (response.status === 501) {
throw new Error(`${response.status}`);
}
throw new Error(await getErrorMsgFromApiResponse(response));
};📝 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
| const handleVulnerabilityJSONResponse = async <R>(response: Response): Promise<R> => { | |
| if (response.ok) { | |
| const data = (await response.json()) as R; | |
| return data; | |
| } | |
| if (response.status === 404) { | |
| throw new Error(`Error ${response.status}: ${response.statusText}`); | |
| } | |
| // API returns 501 for disabled vulnerabilities API. | |
| if (response.status === 501) { | |
| throw new Error(`${response.status}`); | |
| } | |
| throw new Error(await getErrorMsgFromApiResponse(response)); | |
| }; | |
| const handleVulnerabilityJSONResponse = async <R>(response: Response): Promise<R> => { | |
| if (response.ok) { | |
| const data = (await response.json()) as R; | |
| return data; | |
| } | |
| if (response.status === 404) { | |
| throw new Error(`Error ${response.status}: ${response.statusText}`); | |
| } | |
| if (response.status === 401) { | |
| redirectToLogin(); | |
| } | |
| // API returns 501 for disabled vulnerabilities API. | |
| if (response.status === 501) { | |
| throw new Error(`${response.status}`); | |
| } | |
| throw new Error(await getErrorMsgFromApiResponse(response)); | |
| }; |
🤖 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 `@apps/standalone/src/app/utils/apiCalls.ts` around lines 126 - 142,
handleVulnerabilityJSONResponse lacks explicit 401 handling; update the function
(handleVulnerabilityJSONResponse) to mirror handleApiJSONResponse by checking if
response.status === 401, call redirectToLogin() and then throw an appropriate
Error (e.g., using getErrorMsgFromApiResponse(response) or a short message) so
unauthorized vulnerability responses redirect to login instead of producing a
generic error.
rawagner
reviewed
May 14, 2026
Collaborator
rawagner
left a comment
rawagner
left a comment
There was a problem hiding this comment.
just a few minor comments. Otherwise it looks great.
celdrake
force-pushed
the
EDM-3715-vulnerability-management
branch
2 times, most recently
from
4d237d4 to
6c5d2ed
Compare
rawagner
approved these changes
May 15, 2026
celdrake
force-pushed
the
EDM-3715-vulnerability-management
branch
from
6c5d2ed to
f6911d8
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.
New functionality to display the Vulnerabilities present in the system.
