chore: route vulnerability references to canonical URLs#10853
Conversation
- Link fix available versions in finding details for external CVE advisories - Keep Prowler Hub-backed checks on the existing remediation path - Cover the drawer behavior with focused UI tests
|
✅ All necessary |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
🔒 Container Security ScanImage: 📊 Vulnerability Summary
2 package(s) affected
|
- Use external CVE references when a finding has no Hub recommendation - Keep the remediation CTA slot as View in Prowler Hub or View CVE - Cover the CVE drawer behavior with resource detail tests
# Conflicts: # ui/components/findings/table/resource-detail-drawer/resource-detail-drawer-content.tsx
- Resolve CVE recommendations from Trivy references - Remove Aqua advisory URLs from provider metadata - Preserve Prowler Hub remediation links for IaC checks - Cover CVE fallback and non-CVE advisory cases
- Use recommendation URLs as the single CTA source - Keep Prowler Hub and CVE labels distinct - Assert official CVE references are rendered without Aqua URLs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #10853 +/- ##
===========================================
- Coverage 93.65% 70.43% -23.22%
===========================================
Files 230 113 -117
Lines 33937 8345 -25592
===========================================
- Hits 31784 5878 -25906
- Misses 2153 2467 +314
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
🔒 Container Security ScanImage: 📊 Vulnerability Summary
4 package(s) affected
|
- Add `build_finding_reference_url` mapping a finding ID to its canonical reference (`cve.org`, `github.com/advisories`, or `hub.prowler.com/check`) - IAC provider falls back to the helper when no canonical CVE URL is resolved, so misconfigs and non-CVE vulns get a working remediation link instead of an Aqua URL - Strip leading `AVD-` so Prowler Hub URLs resolve, since Hub indexes Trivy rules without the prefix - Cover the helper and IAC behavior with unit tests; refresh changelog entry
- Render "View in Prowler Hub" for hub.prowler.com URLs and "View Advisory" for GitHub Security Advisory URLs alongside the existing "View CVE" action - Fall back to a generic "View Reference" for other destinations - Cover the new label with a unit test; refresh changelog entry
- Move SDK entry from 5.25.0 to 5.26.0 (Prowler UNRELEASED) - Add a 1.26.0 (Prowler UNRELEASED) section in the UI changelog and move the entry there
fd4350c to
de0ddf5
Compare
…lable-links # Conflicts: # ui/CHANGELOG.md
alejandrobailo
left a comment
There was a problem hiding this comment.
The title and description say this PR links each fix-available version when the recommendation points to a CVE advisory. I can't find that change anywhere in the diff. statusExtended, which is where the (fix available: 5.7.13, ...) string lives, still renders as plain text at line 894 of resource-detail-drawer-content.tsx and that block is identical to master.
What the PR does ship is three separate things:
- a dynamic label for the existing recommendation button (
View CVE/View Advisory/View in Prowler Hub/View Reference), - a new
View Resourcelink under the actions menu, - a small refactor so the recommendation button can render without accompanying text.
Only the first one matches the stated goal, and the changelog entry in ui/CHANGELOG.md describes behavior the code doesn't actually have. Either the version linkifier needs to land before merge, or the PR should be retitled and the View Resource piece split out so each change owns its own description, tests, and rollback boundary.
A few notes below.
resource-detail-drawer-content.test.tsx:702-703 (also :749-750)
expect(screen.queryByRole("link", { name: "5.7.13" })).not.toBeInTheDocument();This passes against master too. Nothing in the component creates a link with that accessible name in any state, so the assertion is true regardless of whether a linkifier exists. If you want to pin down "status_extended stays as plain text on a CVE finding", flip it to a positive assertion that the version appears as text inside the statusExtended paragraph. Once the real linkifier lands, you'll want a separate test where the same fixture renders six anchors.
resource-detail-drawer-content.test.tsx:807
expect(screen.queryByText(/avd\.aquasec\.com/)).not.toBeInTheDocument();The fixture never puts an aqua URL into the component, so from the UI's point of view this is vacuously true. The aqua filtering happens in prowler/lib/utils/vulnerability_references.py, and there's already coverage for it on the SDK side. I'd drop this from the drawer test.
resource-detail-drawer-content.tsx:90-128
I'd pull these helpers out of the component file. The drawer is already 1.4k+ lines and these are pure URL utilities with no JSX. They've also got a Python twin (prowler/lib/utils/vulnerability_references.py) that owns the same hostname rules. If both sides are going to maintain the same contract, I'd rather see them in ui/lib/vulnerability-references.ts so the parallel is obvious to whoever touches one next.
A couple of things while you're at it:
isProwlerHubUrl uses startsWith on the full URL while the other two parse with new URL(). That's inconsistent and means https://hub.prowler.com.evil.com/... would match. Switch to new URL(url).hostname === "hub.prowler.com" like the others.
The two try/catch blocks around new URL(url) are the same shape. A safeParseUrl helper would tidy both.
The labels are inline string literals. The repo's TS convention (see ui/CLAUDE.md) is const objects with derived types, e.g.:
const RECOMMENDATION_LINK_LABEL = {
CVE: "View CVE",
HUB: "View in Prowler Hub",
ADVISORY: "View Advisory",
REFERENCE: "View Reference",
} as const;
type RecommendationLinkLabel =
(typeof RECOMMENDATION_LINK_LABEL)[keyof typeof RECOMMENDATION_LINK_LABEL];And the if/if/if cascade reads better as a small rules table you can scan top-to-bottom and extend in one line when the next host shows up.
One more: cve.mitre.org is still a valid CVE URL host that older Trivy data sometimes carries, and cve.org without www can appear too. Worth handling both unless we're certain the SDK normalizes everything to www.cve.org upstream.
resource-detail-drawer-content.tsx:342-346
buildResourceDetailHref for a single query param feels like one line too many. Either drop it inline or, since buildComplianceDetailHref already lives in this file, group both in ui/lib/url-builders.ts so the pattern is consistent.
resource-detail-drawer-content.tsx:465-467
const recommendationUrl =
f?.remediation.recommendation.url ||
checkMeta.remediation.recommendation.url;This shifts the source of the URL from check-level to finding-level. Two findings of the same check can now show different button labels in the drawer if their finding-level URLs point to different advisories. That's probably the intent, but it's a behavior change worth calling out in the changelog. Also, the || means an empty string falls through to checkMeta. If empty string is the canonical "no URL", a small isNonEmptyString check is clearer than relying on truthiness, otherwise the next reader will wonder if it's a typo for ??.
resource-detail-drawer-content.tsx:819-832
View Resource isn't mentioned in the description or the changelog. It works, but I'd move it to its own PR. Mixing it with the recommendation-label change makes the diff harder to bisect if either piece misbehaves later, and it splits the test surface for two unrelated features.
resource-detail-drawer-content.tsx:903-936
When recommendationLink is the only thing inside the remediation card (no text, no CLI, no Terraform, no nativeiac), the card renders as a flex row with gap-3 and a single chip on the right. It looks orphaned. Either keep the Remediation: label even when there's no text, or skip the card entirely when the URL is the only content.
Happy to pair on the refactor if it helps. I'd just rather this come back as either a complete fix-available implementation or a renamed PR that owns what it actually does.
- Move recommendation URL labels to shared UI utilities - Remove unrelated resource navigation from the drawer diff - Keep URL-only remediation cards labeled and update tests
|
Updated in What changed:
Verified locally:
Commit hooks also passed UI TypeScript, ESLint, unit tests, and build. |
Reapplies the View Resource link that was inadvertently dropped while removing this PR's overlap with #10847. That feature is already on master and removing it here would have regressed the findings drawer. Restores buildResourceDetailHref, the resourceDetailHref binding, the JSX action below the resource actions menu, and the original positive assertion in the drawer test.
|
Update on the previous review feedback: when I "removed the unrelated Pushed
Verified |
Set RelatedUrl to an empty string.
3d75111
Context
Trivy image and IaC findings can expose Aqua advisory URLs or finding-specific advisory URLs. This PR routes those findings to canonical public references and makes the finding drawer label the remediation action by destination.
Description
AVD-prefixView CVE,View in Prowler Hub,View Advisory,View Reference), and keep URL-only remediation sections labeledui/lib/vulnerability-references.tsand cover CVE, Prowler Hub, GitHub advisory, malformed, and hostile hostname casesSteps to review
poetry run pytest tests/lib/utils/test_vulnerability_references.py tests/providers/iac/iac_provider_test.py tests/providers/image/image_provider_test.pypnpm test:run lib/vulnerability-references.test.ts components/findings/table/resource-detail-drawer/resource-detail-drawer-content.test.tsxpnpm run typecheckpnpm run lint:checkChecklist
Community Checklist
SDK/CLI
UI (if applicable)
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.