feat(trustify): migrate DA backend from Trustify v2 to v3 endpoints#638
Open
ruromero wants to merge 7 commits into
Open
feat(trustify): migrate DA backend from Trustify v2 to v3 endpoints#638ruromero wants to merge 7 commits into
ruromero wants to merge 7 commits into
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
|
![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 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.
| Issue issue = issues.get(0); | ||
|
|
||
| // Should prioritize V4 based on SCORE_TYPE_ORDER | ||
| // v3 provides pre-computed base_score |
There was a problem hiding this comment.
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:
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:
- Replace
trustifyResponseHandler.handleResponse(response)with the exact helper or setup logic used in the other tests in this class to obtainpackageItemfrom the JSON response. - Ensure the JSON structure (particularly the
packagesandissueslinkage) 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 keepingbase_scorewith only ascorefield and noseverity. - If the class uses a different naming convention for the handler or
PackageItem, adjust the types and variable names accordingly.
|
|
||
| Issue issue = issues.get(0); | ||
| assertEquals("manual", issue.getSource()); | ||
| assertEquals("unknown", issue.getSource()); |
There was a problem hiding this comment.
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:
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:
- 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;
- Adjust the call
TrustifyResponseHandler.responseToIssues(responseWithoutAdvisory);if your handler uses a different factory/utility method or signature (for examplefromResponse,toIssues, or an instance method). - If your Trustify JSON structure requires additional mandatory fields beyond
purlandstatus, extend theresponseWithoutAdvisoryJSON fixture accordingly while keeping theadvisoryfield absent for this test case.
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:
|
ruromero
force-pushed
the
TC-4522
branch
4 times, most recently
from
57e5ce8 to
0f446ab
Compare
Collaborator
Author
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
requested changes
Jul 2, 2026
a-oren
left a comment
a-oren
left a comment
Contributor
There was a problem hiding this comment.
Looks great, just deleting two files that are no longer in use.
Comment on lines
-47
to
-48
| import io.github.guacsec.trustifyda.model.trustify.AdvisoryScore; | ||
| import io.github.guacsec.trustifyda.model.trustify.ScoreType; |
Contributor
There was a problem hiding this comment.
AdvisoryScore.java and ScoreType.java are no longer imported or used anywhere in the codebase after this PR, delete both files
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>
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
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: