feat(report): display remediation details and advisory info in HTML report (TC-4523)#643
Open
ruromero wants to merge 4 commits into
Open
feat(report): display remediation details and advisory info in HTML report (TC-4523)#643ruromero wants to merge 4 commits into
ruromero wants to merge 4 commits into
Conversation
…eport (TC-4523) Add RemediationDetails component to render remediation entries with category labels, details links, and advisory information when no fixedIn version is available. The component slots into the existing priority chain: trustedContent > fixedIn > remediations > no remediations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Reviewer's GuideThis PR extends the HTML vulnerability report to show structured remediation details (category labels, linked details, and advisory info) and wires those into the existing remediation priority chain, backed by new TypeScript types and an integration test for the rendered DOM. Flow diagram for remediation rendering priority in VulnerabilityRowflowchart LR
start([VulnerabilityRow render]) --> hasTrustedContent{remediation.trustedContent}
hasTrustedContent -->|yes| showTrusted[Render RemediationLink]
hasTrustedContent -->|no| hasFixedIn{remediation.fixedIn}
hasFixedIn -->|yes| showFixedIn[Render VulnerabilityLink]
hasFixedIn -->|no| hasRemediations{remediation.remediations length > 0}
hasRemediations -->|yes| showDetails[Render RemediationDetails]
hasRemediations -->|no| hasAnyRemediation{hasRemediations helper}
hasAnyRemediation -->|no| showEmpty[Render empty span]
hasAnyRemediation -->|yes| endNode([Render nothing])
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}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
VulnerabilityRowrender logic is getting hard to follow with the nested ternary chain; consider extracting the remediation selection into a small helper function or child component to make the priority order (trustedContent > fixedIn > remediations) clearer and easier to maintain. - In
RemediationDetails, using the array index as thekeycan lead to unnecessary re-renders or subtle bugs if the data changes order; if possible, derive a more stable key (e.g., from advisory id + category + url) to improve React reconciliation. - The
hasRemediationsfunction and theVulnerabilityRowboth independently check forremediation.remediations?.length > 0; consider centralizing that logic inhasRemediationsand relying on it in the component to avoid duplication and keep the remediation presence rules in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `VulnerabilityRow` render logic is getting hard to follow with the nested ternary chain; consider extracting the remediation selection into a small helper function or child component to make the priority order (trustedContent > fixedIn > remediations) clearer and easier to maintain.
- In `RemediationDetails`, using the array index as the `key` can lead to unnecessary re-renders or subtle bugs if the data changes order; if possible, derive a more stable key (e.g., from advisory id + category + url) to improve React reconciliation.
- The `hasRemediations` function and the `VulnerabilityRow` both independently check for `remediation.remediations?.length > 0`; consider centralizing that logic in `hasRemediations` and relying on it in the component to avoid duplication and keep the remediation presence rules in one place.
## Individual Comments
### Comment 1
<location path="ui/src/components/RemediationDetails.tsx" line_range="17-26" />
<code_context>
+export const RemediationDetails: React.FC<RemediationDetailsProps> = ({ remediations }) => {
+ return (
+ <>
+ {remediations.map((rem, index) => {
+ const label = rem.category
+ ? categoryLabels[rem.category] ?? rem.category
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Use a more stable key than the array index for mapped remediations
Using the array index as a React key can cause subtle UI bugs when the list is reordered or items are added/removed. Prefer a stable identifier (e.g., combining `rem.advisory?.id`, `rem.category`, or `rem.url`) so React can correctly track item identity.
```suggestion
export const RemediationDetails: React.FC<RemediationDetailsProps> = ({ remediations }) => {
return (
<>
{remediations.map((rem, index) => {
const label = rem.category
? categoryLabels[rem.category] ?? rem.category
: undefined;
const remediationKeyParts = [
rem.advisory?.id,
rem.category,
rem.url,
].filter(Boolean) as string[];
const remediationKey =
remediationKeyParts.length > 0
? remediationKeyParts.join('|')
: `remediation-${index}`;
return (
<div key={remediationKey}>
```
</issue_to_address>
### Comment 2
<location path="ui/src/components/RemediationDetails.tsx" line_range="32-41" />
<code_context>
+ <span>
+ {label ? ': ' : ''}
+ {rem.url ? (
+ <a href={rem.url} target="_blank" rel="noreferrer">
+ {rem.details}
+ </a>
+ ) : (
+ rem.details
+ )}
+ </span>
+ )}
+ {!rem.details && rem.url && (
+ <span>
+ {label ? ': ' : ''}
+ <a href={rem.url} target="_blank" rel="noreferrer">
+ Details
+ </a>
+ </span>
+ )}
+ {rem.advisory && (
+ <div>
+ {rem.advisory.url ? (
+ <a href={rem.advisory.url} target="_blank" rel="noreferrer">
+ {rem.advisory.title || rem.advisory.id}
+ </a>
</code_context>
<issue_to_address>
**🚨 issue (security):** Include `noopener` in `rel` for `target="_blank"` links to avoid opener access
These `target="_blank"` anchors only use `rel="noreferrer"`. Without `noopener`, the new page can access `window.opener`. Please add `noopener` (e.g., `rel="noreferrer noopener"`) to these links.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+17
to
+26
| export const RemediationDetails: React.FC<RemediationDetailsProps> = ({ remediations }) => { | ||
| return ( | ||
| <> | ||
| {remediations.map((rem, index) => { | ||
| const label = rem.category | ||
| ? categoryLabels[rem.category] ?? rem.category | ||
| : undefined; | ||
|
|
||
| return ( | ||
| <div key={index}> |
There was a problem hiding this comment.
suggestion (bug_risk): Use a more stable key than the array index for mapped remediations
Using the array index as a React key can cause subtle UI bugs when the list is reordered or items are added/removed. Prefer a stable identifier (e.g., combining rem.advisory?.id, rem.category, or rem.url) so React can correctly track item identity.
Suggested change
| export const RemediationDetails: React.FC<RemediationDetailsProps> = ({ remediations }) => { | |
| return ( | |
| <> | |
| {remediations.map((rem, index) => { | |
| const label = rem.category | |
| ? categoryLabels[rem.category] ?? rem.category | |
| : undefined; | |
| return ( | |
| <div key={index}> | |
| export const RemediationDetails: React.FC<RemediationDetailsProps> = ({ remediations }) => { | |
| return ( | |
| <> | |
| {remediations.map((rem, index) => { | |
| const label = rem.category | |
| ? categoryLabels[rem.category] ?? rem.category | |
| : undefined; | |
| const remediationKeyParts = [ | |
| rem.advisory?.id, | |
| rem.category, | |
| rem.url, | |
| ].filter(Boolean) as string[]; | |
| const remediationKey = | |
| remediationKeyParts.length > 0 | |
| ? remediationKeyParts.join('|') | |
| : `remediation-${index}`; | |
| return ( | |
| <div key={remediationKey}> |
Comment on lines
+32
to
+41
| <a href={rem.url} target="_blank" rel="noreferrer"> | ||
| {rem.details} | ||
| </a> | ||
| ) : ( | ||
| rem.details | ||
| )} | ||
| </span> | ||
| )} | ||
| {!rem.details && rem.url && ( | ||
| <span> |
There was a problem hiding this comment.
🚨 issue (security): Include noopener in rel for target="_blank" links to avoid opener access
These target="_blank" anchors only use rel="noreferrer". Without noopener, the new page can access window.opener. Please add noopener (e.g., rel="noreferrer noopener") to these links.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #643 +/- ##
============================================
+ Coverage 57.19% 57.23% +0.03%
Complexity 878 878
============================================
Files 92 92
Lines 5021 5027 +6
Branches 691 692 +1
============================================
+ Hits 2872 2877 +5
Misses 1841 1841
- Partials 308 309 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…overs (TC-4523) Replace the mutually exclusive if/else cascade in VulnerabilityRow with independent conditional renders so trustedContent, fixedIn, and remediations can all appear in the same cell. Show actual fixedIn version strings instead of a redundant CVE-to-deps.dev link. Render remediations as PatternFly Popover badges with clickable category labels expanding to full details. Replace provider/unknown with provider/Other. Remove dead remediationLink utility function. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… rewriting (TC-4523) - Fix getSourceName() to replace literal "unknown" strings from the backend with "Other" - Add advisoryLink() utility to rewrite advisory URLs using branding config template - Add advisoryIssueTemplate to BrandingConfig interface and backend config mapping - Skip redundant Popover when remediation details duplicate vulnerability title - Improve remediation vertical spacing with div wrappers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The advisoryLink() rewriter was only applied to advisory.url but not to the remediation URL itself. This caused broken links like https://www.redhat.com/#CVE-2023-2454 to remain unrewritten even when branding.advisory-issue-template was configured. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.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
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.
Summary
RemediationDetailsReact component that renders remediation entries with category labels (Workaround,Vendor Fix, etc.), details links, and advisory informationVulnerabilityRowpriority chain: trustedContent > fixedIn > remediations > no remediationsRemediationCategory,AdvisoryInfo,RemediationInfo) toreport.tsand updatehasRemediations()to include the newremediationsfieldtestHtmlRemediationDetailsintegration test verifying CVE-2022-42003 renders workaround category, details link, and advisory titleJira
TC-4523
Test plan
mvn verify— all 341 tests passyarn lint— no lint errorsyarn build— UI builds successfullymvn spotless:apply— code formattedtestHtmlRemediationDetailsvalidates DOM structure🤖 Generated with Claude Code
Summary by Sourcery
Display detailed remediation information in HTML vulnerability reports and surface it when no fixed version or trusted content is available.
New Features:
Tests: