feat(trustify): migrate DA backend from Trustify v2 to v3 endpoints#638
Conversation
Reviewer's GuideMigrates DA backend Trustify integration from v2 to v3 analyze/recommend endpoints and rewrites response handling logic and tests to consume the new v3 response shape (base_score, purl_statuses, advisory.issuer, remediations, version ranges). Sequence diagram for Trustify v3 analyze endpoint integrationsequenceDiagram
participant DAService
participant TrustifyApi
participant TrustifyResponseHandler
participant Issue
DAService->>TrustifyApi: POST /api/v3/vulnerability/analyze
TrustifyApi-->>DAService: v3 response (details, purl_statuses, base_score)
DAService->>TrustifyResponseHandler: toIssues(response)
TrustifyResponseHandler->>TrustifyResponseHandler: getSource(purl_status)
TrustifyResponseHandler->>TrustifyResponseHandler: setCvssData(issue, vuln, purlStatus)
TrustifyResponseHandler->>Issue: cvssScore(base_score.score)
TrustifyResponseHandler->>Issue: setSeverity(SeverityUtils.fromValue | fromScore)
TrustifyResponseHandler->>Issue: setRemediation(Remediation)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The new
setCvssDatalogic relies solely onbase_score; if Trustify ever omits this field for some entries, consider a fallback to per-advisory/purl_status scoring data to avoid silently producing issues without CVSS information. - The remediation construction in
setCvssData(version range + remediations) has become quite dense; consider extracting this into helper methods to make the transformation from v3 JSON toRemediationandVersionRangeeasier to follow and maintain. - Changing
DEFAULT_SOURCEfrom "manual" to "unknown" and deriving source fromissuer.nameinstead of the previous importer may affect how issues are grouped by source; if this is intentional, it may be worth centralizing the mapping logic in a dedicated helper to keep future adjustments localized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `setCvssData` logic relies solely on `base_score`; if Trustify ever omits this field for some entries, consider a fallback to per-advisory/purl_status scoring data to avoid silently producing issues without CVSS information.
- The remediation construction in `setCvssData` (version range + remediations) has become quite dense; consider extracting this into helper methods to make the transformation from v3 JSON to `Remediation` and `VersionRange` easier to follow and maintain.
- Changing `DEFAULT_SOURCE` from "manual" to "unknown" and deriving source from `issuer.name` instead of the previous importer may affect how issues are grouped by source; if this is intentional, it may be worth centralizing the mapping logic in a dedicated helper to keep future adjustments localized.
## Individual Comments
### Comment 1
<location path="src/test/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyResponseHandlerTest.java" line_range="472" />
<code_context>
Issue issue = issues.get(0);
- // Should prioritize V4 based on SCORE_TYPE_ORDER
+ // v3 provides pre-computed base_score
assertEquals(7.2f, issue.getCvssScore());
assertEquals(Severity.HIGH, issue.getSeverity());
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test where base_score has a score but no severity to verify score-based severity fallback
The updated `setCvssData` falls back to `SeverityUtils.fromScore` when `base_score.severity` is missing or unparsable, but there’s no test for a `base_score` that has only a numeric score and no severity field. Please add a test (e.g., a variant of `testResponseToIssuesWithMultipleScoreTypes`) using such input to verify that severity is correctly derived from the score when Trustify omits `severity`.
Suggested implementation:
```java
List<Issue> issues = packageItem.issues();
Issue issue = issues.get(0);
// v3 provides pre-computed base_score
assertEquals(7.2f, issue.getCvssScore());
assertEquals(Severity.HIGH, issue.getSeverity());
}
@Test
void testBaseScoreSeverityFallbackWhenSeverityMissing() throws Exception {
String response = """
{
"vulnerabilities": [{
"identifier": "CVE-2024-1597",
"title": "Test CVE",
"scores": {
"cvss_v3": {
"version": "3.1",
"vector": "CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:H/I:H/A:H",
"base_score": {
"score": 7.2
}
}
}
}],
"packages": [{
"purl": "pkg:maven/org.example/foo@1.0.0",
"issues": [{
"identifier": "CVE-2024-1597"
}]
}],
"warnings": []
}
""";
// Adapt this setup to however the existing tests obtain a PackageItem
PackageItem packageItem = trustifyResponseHandler.handleResponse(response);
List<Issue> issuesWithScoreOnly = packageItem.issues();
Issue issueWithScoreOnly = issuesWithScoreOnly.get(0);
// base_score has only a numeric score and no severity; severity should be derived from the score
assertEquals(7.2f, issueWithScoreOnly.getCvssScore());
assertEquals(Severity.HIGH, issueWithScoreOnly.getSeverity());
}
{
"identifier": "CVE-2024-1597",
```
The new test assumes there is a `trustifyResponseHandler.handleResponse(String)` (or similar) method that returns a `PackageItem`, consistent with how `packageItem` is created in the existing tests. To integrate this test correctly, you should:
1. Replace `trustifyResponseHandler.handleResponse(response)` with the exact helper or setup logic used in the other tests in this class to obtain `packageItem` from the JSON response.
2. Ensure the JSON structure (particularly the `packages` and `issues` linkage) matches what the existing tests expect. If the existing tests use a slightly different envelope (e.g., different top-level keys or nesting), adjust the JSON in the new test to mirror that structure while keeping `base_score` with only a `score` field and no `severity`.
3. If the class uses a different naming convention for the handler or `PackageItem`, adjust the types and variable names accordingly.
</issue_to_address>
### Comment 2
<location path="src/test/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyResponseHandlerTest.java" line_range="699" />
<code_context>
Issue issue = issues.get(0);
- assertEquals("manual", issue.getSource());
+ assertEquals("unknown", issue.getSource());
}
</code_context>
<issue_to_address>
**suggestion (testing):** Consider a dedicated test for purl_status entries without an advisory to exercise getSource’s DEFAULT_SOURCE path explicitly
Since `getSource` now derives the source from `advisory.issuer.name` with a fallback to "unknown", this test only covers the `issuer == null` case when an advisory is present. Please also add a test where `purl_status` has no `advisory` at all to cover the `advisory == null` branch and verify it still returns "unknown" without throwing.
Suggested implementation:
```java
import io.github.guacsec.trustifyda.api.PackageRef;
import io.github.guacsec.trustifyda.api.v5.Issue;
import io.github.guacsec.trustifyda.api.v5.RemediationCategory;
import io.github.guacsec.trustifyda.api.v5.Severity;
import io.github.guacsec.trustifyda.integration.Constants;
import io.github.guacsec.trustifyda.integration.providers.trustify.ubi.UBIRecommendation;
@Test
void testIssueSourceIsUnknownWhenAdvisoryIsMissing() {
// Response containing a purl_status entry without an advisory block to
// exercise the getSource DEFAULT_SOURCE ("unknown") path.
String responseWithoutAdvisory = """
{
"purl_status": [
{
"purl": "pkg:maven/org.example/foo@1.0.0",
"status": "vulnerable"
}
]
}
""";
List<Issue> issues = TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);
assertEquals(1, issues.size());
Issue issue = issues.get(0);
assertEquals("unknown", issue.getSource());
}
private static Stream<String> testResponseToIssuesWithValidData() {
return Stream.of(
```
Depending on the existing code in `TrustifyResponseHandlerTest` and `TrustifyResponseHandler`, you may need to:
1. Ensure the following imports are present at the top of the file (if they are not already):
- `import java.util.List;`
- `import org.junit.jupiter.api.Test;`
2. Adjust the call `TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);` if your handler uses a different factory/utility method or signature (for example `fromResponse`, `toIssues`, or an instance method).
3. If your Trustify JSON structure requires additional mandatory fields beyond `purl` and `status`, extend the `responseWithoutAdvisory` JSON fixture accordingly while keeping the `advisory` field absent for this test case.
</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 #638 +/- ##
============================================
- Coverage 56.86% 56.67% -0.20%
- Complexity 823 836 +13
============================================
Files 94 92 -2
Lines 4815 4863 +48
Branches 624 655 +31
============================================
+ Hits 2738 2756 +18
- Misses 1802 1814 +12
- Partials 275 293 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
57e5ce8 to
0f446ab
Compare
Verification Report for TC-4522 (commit a007ebb)
Overall: WARNTwo warnings to note:
No blocking issues found. All acceptance criteria are met and CI passes. This comment was AI-generated by sdlc-workflow/verify-pr v0.11.0. |
a-oren
left a comment
There was a problem hiding this comment.
Looks great, just deleting two files that are no longer in use.
Cover untested code paths in setCvssData, getSource, and toIssues: fallback to purlStatus scores, full version range fields, remediation URL, unknown remediation category, missing/blank advisory source, score-based severity derivation, and CVE deduplication logic. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stedContent RecommendationAggregation and RegistryEnrichmentService were overwriting the Remediation object when adding trustedContent, destroying any upstream fixedIn/versionRanges/remediations data from Trustify v3. Now merges onto the existing Remediation instead. Also updates the remediation counter in ProviderResponseHandler to count upstream-only remediations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When highInclusive is true, the high version is still affected — it should not appear in fixedIn. Read highInclusive before deciding whether to populate the fixed version list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
TC-4522 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Migrates the DA backend from Trustify v2
/api/v2/vulnerability/analyzeand/api/v2/purl/recommendendpoints to v3/api/v3/vulnerability/analyzeand/api/v3/purl/recommend.TrustifyResponseHandlerto parse the v3 response format (details[].purl_statuses[],base_score,advisory.issuer)trustify-da-api-modelto2.0.10-SNAPSHOTTest plan
mvn verify -Pdev)TrustifyResponseHandlerTest— 12 test cases cover: empty responses, single/multiple issues, severity mapping, remediation parsing, withdrawn advisories, CVE deduplication, unknown severity fallbackAnalysisTestintegration tests pass with v3 fixturesJira
TC-4522
Dependencies
Summary by Sourcery
Migrate Trustify integration to the v3 vulnerability and recommendation APIs and adapt response handling to the new schema.
New Features:
Enhancements:
Build:
Tests: