feat(advisory): CSAF VEX Visualizer — all tabs combined [TC-4612]#1070
Draft
CryptoRodeo wants to merge 3 commits into
Draft
feat(advisory): CSAF VEX Visualizer — all tabs combined [TC-4612]#1070CryptoRodeo wants to merge 3 commits into
CryptoRodeo wants to merge 3 commits into
Conversation
Contributor
Reviewer's GuideImplements a CSAF-specific advisory details experience: defines CSAF 2.0 TypeScript types, adds a CSAF JSON fetch hook, wires CSAF type detection into advisory details to switch to a new 5-tab VEX visualizer, and introduces several CSAF-focused UI components including overview metrics, per-CVE details, product and relationship trees using ECharts, and a raw JSON source view. Sequence diagram for CSAF document fetch and visualizationsequenceDiagram
actor User
participant AdvisoryDetails
participant CsafAdvisoryDetails
participant useFetchAdvisoryCsafById
participant downloadAdvisory
participant CsafTabs
User->>AdvisoryDetails: open advisory details
AdvisoryDetails->>AdvisoryDetails: compute isCsaf
AdvisoryDetails->>CsafAdvisoryDetails: render(advisoryId) [isCsaf]
CsafAdvisoryDetails->>useFetchAdvisoryCsafById: call(advisoryId)
useFetchAdvisoryCsafById->>downloadAdvisory: downloadAdvisory(client, path)
downloadAdvisory-->>useFetchAdvisoryCsafById: Blob response
useFetchAdvisoryCsafById->>useFetchAdvisoryCsafById: blob.text() + JSON.parse
useFetchAdvisoryCsafById-->>CsafAdvisoryDetails: { csafDocument, isFetching, fetchError }
CsafAdvisoryDetails->>CsafTabs: render with csafDocument
CsafTabs-->>User: CSAF overview, vulnerabilities, trees, source
Flow diagram for CSAF vs default advisory details layoutflowchart LR
AdvisoryDetails["AdvisoryDetails component"] --> FetchAdvisory["useFetchAdvisoryById(advisoryId)"]
FetchAdvisory --> AdvisoryLoaded["advisory loaded"]
AdvisoryLoaded --> CheckType{"advisory.labels.type === csaf"}
CheckType -->|true| CsafDetails["CsafAdvisoryDetails(advisoryId)"]
CheckType -->|false| DefaultTabs["Overview + Vulnerabilities tabs"]
CsafDetails --> CsafTabs["5 CSAF tabs (overview, vulnerabilities, product_tree, relationship_tree, source)"]
DefaultTabs --> NonCsafTabs["2 default tabs (info, vulnerabilities)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
CsafAdvisoryDetails, the tab content refs are created withReact.createRef()inside the component body, which recreates them on each render; consider switching touseRefso the refs are stable across renders and behave correctly withtabContentRef. - The
LinkifiedTextcomponent incsaf-remediations.tsxuses a global regex (/g) and then callsurlRegex.test(part)insidemap, which will give incorrect results becausetestadvanceslastIndex; either remove the global flag or use a fresh regex/test per part. - In
useFetchAdvisoryCsafById, JSON parse failures will be surfaced as non-Axios errors but are typed asAxiosError; consider wrappingJSON.parsein a try/catch and either throwing anAxiosError-compatible shape or widening thefetchErrortype to reflect non-network parsing errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `CsafAdvisoryDetails`, the tab content refs are created with `React.createRef()` inside the component body, which recreates them on each render; consider switching to `useRef` so the refs are stable across renders and behave correctly with `tabContentRef`.
- The `LinkifiedText` component in `csaf-remediations.tsx` uses a global regex (`/g`) and then calls `urlRegex.test(part)` inside `map`, which will give incorrect results because `test` advances `lastIndex`; either remove the global flag or use a fresh regex/test per part.
- In `useFetchAdvisoryCsafById`, JSON parse failures will be surfaced as non-Axios errors but are typed as `AxiosError`; consider wrapping `JSON.parse` in a try/catch and either throwing an `AxiosError`-compatible shape or widening the `fetchError` type to reflect non-network parsing errors.
## Individual Comments
### Comment 1
<location path="client/src/app/pages/advisory-details/advisory-details.tsx" line_range="92" />
<code_context>
useDeleteAdvisoryMutation(onDeleteAdvisorySuccess, onDeleteAdvisoryError);
- // Tabs
+ const isCsaf = advisory?.labels.type === "csaf";
+
+ // Tabs (default non-CSAF layout)
</code_context>
<issue_to_address>
**issue:** Guard against `labels` being undefined when deriving `isCsaf`.
`advisory?.labels.type` still throws if `advisory` exists but `labels` is null/undefined. Use `const isCsaf = advisory?.labels?.type === "csaf";` so both `advisory` and `labels` are safely optional.
</issue_to_address>
### Comment 2
<location path="client/src/app/pages/advisory-details/csaf-overview.tsx" line_range="178-187" />
<code_context>
+ )}
+ {cvss && (
+ <FlexItem>
+ <SeverityShieldAndText
+ value={
+ cvss.baseSeverity.toLowerCase() as
+ | "critical"
+ | "important"
+ | "moderate"
+ | "low"
+ | "none"
+ }
+ score={cvss.baseScore}
+ />
+ </FlexItem>
+ )}
</code_context>
<issue_to_address>
**issue (bug_risk):** Align CSAF severities with the `SeverityShieldAndText` expected values.
Here and in the impact summary, CSAF/CVSS severities (`CRITICAL`, `HIGH`, `MEDIUM`, etc.) are just lowercased and passed to `SeverityShieldAndText`, which expects only `"critical" | "important" | "moderate" | "low" | "none"`. That means `"high"` and `"medium"` won’t map correctly. Please normalize the CSAF severities first (e.g. `HIGH -> important`, `MEDIUM -> moderate`, and any unknown value -> `none`) before passing them into `SeverityShieldAndText`.
</issue_to_address>
### Comment 3
<location path="client/src/app/pages/advisory-details/components/csaf-vulnerability-card.tsx" line_range="171-155" />
<code_context>
+ )}
+
+ {/* References */}
+ {vulnerability.references && vulnerability.references.length > 0 && (
+ <FlexItem>
+ <ExpandableSection toggleText="References" isExpanded={false}>
+ <List isPlain>
+ {vulnerability.references.map((ref, i) => (
+ <ListItem key={`ref-${i}`}>
+ <a
+ href={ref.url}
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ {ref.summary || ref.url} <ExternalLinkAltIcon />
+ </a>
+ </ListItem>
+ ))}
+ </List>
+ </ExpandableSection>
+ </FlexItem>
+ )}
</code_context>
<issue_to_address>
**issue (bug_risk):** The "References" expandable section is permanently collapsed.
This `ExpandableSection` is hard-coded with `isExpanded={false}` and no `onToggle`, so the references are never visible. Either let it be uncontrolled by omitting `isExpanded`, or control `isExpanded` via component state so the user can expand it.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| useDeleteAdvisoryMutation(onDeleteAdvisorySuccess, onDeleteAdvisoryError); | ||
|
|
||
| // Tabs | ||
| const isCsaf = advisory?.labels.type === "csaf"; |
Contributor
There was a problem hiding this comment.
issue: Guard against labels being undefined when deriving isCsaf.
advisory?.labels.type still throws if advisory exists but labels is null/undefined. Use const isCsaf = advisory?.labels?.type === "csaf"; so both advisory and labels are safely optional.
Comment on lines
+178
to
+187
| <SeverityShieldAndText | ||
| value={ | ||
| doc.aggregate_severity.text.toLowerCase() as | ||
| | "critical" | ||
| | "important" | ||
| | "moderate" | ||
| | "low" | ||
| | "none" | ||
| } | ||
| score={null} |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Align CSAF severities with the SeverityShieldAndText expected values.
Here and in the impact summary, CSAF/CVSS severities (CRITICAL, HIGH, MEDIUM, etc.) are just lowercased and passed to SeverityShieldAndText, which expects only "critical" | "important" | "moderate" | "low" | "none". That means "high" and "medium" won’t map correctly. Please normalize the CSAF severities first (e.g. HIGH -> important, MEDIUM -> moderate, and any unknown value -> none) before passing them into SeverityShieldAndText.
| +{hiddenCount} more | ||
| </Button> | ||
| )} | ||
| </ExpandableSection> |
Contributor
There was a problem hiding this comment.
issue (bug_risk): The "References" expandable section is permanently collapsed.
This ExpandableSection is hard-coded with isExpanded={false} and no onToggle, so the references are never visible. Either let it be uncontrolled by omitting isExpanded, or control isExpanded via component state so the user can expand it.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1070 +/- ##
==========================================
- Coverage 49.17% 46.43% -2.74%
==========================================
Files 253 264 +11
Lines 5499 5819 +320
Branches 1660 1790 +130
==========================================
- Hits 2704 2702 -2
- Misses 2519 2842 +323
+ Partials 276 275 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
CryptoRodeo
force-pushed
the
TC-4612-full-changes
branch
from
bcd547b to
ce8a3c6
Compare
Signed-off-by: Bryan Ramos <bramos@redhat.com>
CryptoRodeo
force-pushed
the
TC-4612-full-changes
branch
from
ce8a3c6 to
d30eccd
Compare
manual fix. Signed-off-by: Bryan Ramos <bramos@redhat.com>
- Replace custom severity bars with PatternFly ChartBar showing affected products per severity - Restructure overview card with document title as header and severity/TLP badges - Add two-column grid layout to vulnerability cards - Link CVE tracking IDs to vulnerability details - Use human-readable date format throughout - Move tree legends above charts with usage hints, expand relationship legend with edge styles - Rename Product Tree tab to Products Signed-off-by: Bryan Ramos <bramos@redhat.com>
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
1 participant
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.
Summary
Combined PR with all changes for the CSAF VEX Visualizer feature (TC-4612). This merges all 7 implementation task branches for testing the complete feature as a single unit.
Changes included:
useFetchAdvisoryCsafByIdquery hook, ECharts dependencylabels.type === "csaf") and conditional 5-tab layoutNew files (15):
client/src/app/types/csaf.tsclient/src/app/pages/advisory-details/csaf-advisory-details.tsxclient/src/app/pages/advisory-details/csaf-overview.tsxclient/src/app/pages/advisory-details/csaf-vulnerabilities.tsxclient/src/app/pages/advisory-details/csaf-product-tree.tsxclient/src/app/pages/advisory-details/csaf-relationship-tree.tsxclient/src/app/pages/advisory-details/csaf-source.tsxclient/src/app/pages/advisory-details/components/csaf-vulnerability-card.tsxclient/src/app/pages/advisory-details/components/csaf-cvss-details.tsxclient/src/app/pages/advisory-details/components/csaf-remediations.tsxclient/src/app/pages/advisory-details/helpers/csaf-tree-helpers.tsclient/src/app/pages/advisory-details/helpers/csaf-relationship-helpers.tsModified files:
client/src/app/queries/advisories.ts— addeduseFetchAdvisoryCsafByIdhookclient/src/app/pages/advisory-details/advisory-details.tsx— CSAF type detection + conditional renderingclient/package.json— addedecharts,echarts-for-reactTest plan
Implements TC-4612
Assisted-by: Claude Code
Summary by Sourcery
Add a CSAF-specific advisory details experience with a dedicated 5-tab VEX visualizer and typed CSAF document support.
New Features:
Enhancements:
Build: