feat(report): improve remediation display with GHSA links and version range popovers (TC-4523) - #643
Merged
Merged
Conversation
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.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #643 +/- ##
============================================
+ Coverage 57.19% 57.66% +0.46%
- Complexity 878 918 +40
============================================
Files 92 93 +1
Lines 5021 5107 +86
Branches 690 721 +31
============================================
+ Hits 2872 2945 +73
- Misses 1841 1842 +1
- Partials 308 320 +12
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
ruromero
changed the title
ruromero
force-pushed
the
TC-4523
branch
3 times, most recently
from
July 17, 2026 05:59
db98ae6 to
d74d89d
Compare
Collaborator
Author
Verification Report for TC-4523 (commit 5cfefe0)
Overall: FAILScope Containment FAIL: The task specification lists Commit Traceability WARN: Commit Test Documentation WARN: This comment was AI-generated by sdlc-workflow/verify-pr v0.13.2. |
Collaborator
Author
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review (ID: 4670658967) Classified as:
|
Collaborator
Author
Verification Report for TC-4523 (commit 1d6b413)
Overall: WARNImproved from prior run's FAIL. All WARNs are informational — no blocking issues remain. Scope, diff size, and commit traceability warnings reflect the feature's natural complexity (backend handler refactoring + test fixtures + React UI). Test documentation warning is minor (comment style preference). This comment was AI-generated by sdlc-workflow/verify-pr v0.13.2. |
a-oren
requested changes
Jul 19, 2026
…SA links (TC-4523) Restructure the remediation column to display advisory-grouped data: - Group remediations by advisory with version range context - Derive fixedIn versions from version ranges (highVersion where !highInclusive) - Add GHSA advisory link generation in TrustifyResponseHandler - Add VersionComparator for semantic version sorting - Replace RemediationDetails with AdvisoryRemediations component - Fix duplicate CVE rows via mergeRemediation dedup in buildVulnerabilityItems - Rewrite advisory link URLs through branding config - Bump trustify-da-api dependency to 2.0.12-SNAPSHOT Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…er changes The batch report fixture was not updated when TrustifyResponseHandler was restructured, causing testBatchSBOMAllWithToken to fail for both CYCLONEDX and SPDX formats. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hasRemediations (TC-4523) - Add rel="noreferrer noopener" to all target="_blank" links in RemediationDetails for reverse-tabnabbing protection - Replace array-index keys with composite keys derived from advisory id, category, and url for stable React reconciliation - Update hasRemediations() to also check non-empty remediations[] within advisories, and use it as a guard in VulnerabilityRow instead of duplicating the inline check Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…TC-4523) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s and advisories (TC-4523) - Remove unused RemediationDetails.tsx component - Remove stale advisory field from RemediationInfo interface - Extract compareVersions and formatRange to shared utils/version.ts - Deduplicate advisories by ID in mergeRemediation - Replace array index key with composite key in AdvisoryRemediations Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Read advisory.labels["importer"] instead of advisory.issuer.name to display the actual data source (e.g. "redhat-csaf", "github-osv"). Fall back to issuer name, then to "manual" for manually uploaded advisories. The UI maps "manual" and "unknown" to "Other". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Group FixedInEntry by version across all advisories so that each version appears once with all advisory details in its popover, instead of showing duplicate entries when multiple advisories share a fixed-in version. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…itle (TC-4523) The UI now displays advisory IDs in the popover instead of titles, so the HtmlReportTest assertion needs to match GHSA-jjjh-jjxp-wpff rather than the full advisory title text. 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
3 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
buildAdvisoryInfo()remediation.detailsin popover body instead of advisory title; fall back to plain advisory link when no details are availableadvisoryIssueTemplatetemplate param to fix vendor-specific CVE link rewritingTest plan
TrustifyResponseHandlerTest— 34/34 pass (includes GHSA URL generation assertion)HtmlReportTest— advisory link rewriting verifiedgithub.com/advisories/...Jira: TC-4523
🤖 Generated with Claude Code