Skip to content

Correlate Go vulnerabilities when OSS Index normalizes version prefix - #7757

Open
jha-pravin wants to merge 2 commits into
DependencyTrack:mainfrom
jha-pravin:feature/oss-index-go-version-normalization
Open

jha-pravin wants to merge 2 commits into
DependencyTrack:mainfrom
jha-pravin:feature/oss-index-go-version-normalization

Conversation

@jha-pravin

Copy link
Copy Markdown
Contributor

Description

Fixes an issue where the OSS Index vulnerability analyzer drops vulnerability findings for Go packages when OSS Index returns coordinates with a normalized version prefix (e.g., pkg:golang/golang.org/x/text@v0.3.5 in response to submitted pkg:golang/golang.org/x/text@0.3.5).

Previously, OssIndexVulnAnalyzer performed only exact string matching between submitted PURL coordinates and returned report coordinates. When coordinates differed due to version prefix normalization, DT logged a warning that vulnerabilities were received for an unsubmitted PURL, dropped the findings, and stored a negative cache entry under the submitted PURL.

This PR implements two-pass correlation following maintainer guidance:

  1. Exact string coordinate matching first.
  2. Version-aware matching for unmatched entries using versatile (VersionFactory.forScheme(...) / KnownVersioningSchemes), which treats Go versions with and without the v prefix as equal.
  3. Results are cached under both the submitted PURL and the report coordinates so that neither entry suffers from false negative cache poisoning.

Addressed Issue

Fixes #7739

Additional Details

  • Added dependency io.github.nscuro:versatile-core to vuln-analysis/oss-index/pom.xml (version managed in parent POM).
  • Added arePurlsEquivalent helper in OssIndexVulnAnalyzer checking type, namespace, name, subpath, qualifiers, and version equality via versatile.
  • Stored reports in entriesToCache under both submitted PURL and returned report coordinates.
  • Added comprehensive unit tests in OssIndexVulnAnalyzerTest:
    • shouldCorrelateGoVulnerabilitiesWhenOssIndexNormalizesVersionWithPrefix: verifies correlation, VDR bom-ref attachment, and subsequent cache hits for both submitted and returned coordinates.
    • shouldCacheNegativeResultWhenGoComponentHasNoVulnerabilitiesAndOssIndexNormalizesVersion: verifies negative caching behavior when no vulnerabilities are present.

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have updated the migration changelog accordingly
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly
  • This PR is a substantial change (per the ADR criteria), and I have added an ADR under docs/adr/

When querying OSS Index for Go packages without a 'v' prefix (e.g., pkg:golang/golang.org/x/text@0.3.5), OSS Index returns component reports where coordinates have been normalized with a 'v' prefix (e.g., pkg:golang/golang.org/x/text@v0.3.5).

Previously, the OSS Index analyzer relied only on exact string matching between submitted PURL coordinates and returned coordinates. Because the coordinates differed, findings were dropped, a warning was logged indicating no component with that PURL was submitted, and a negative cache entry was written under the submitted PURL.

Implement two-pass correlation in OssIndexVulnAnalyzer:
1. Exact coordinate matching pass.
2. Version-aware matching pass for unmatched entries using versatile (which ignores 'v' prefixes in Go versions).

Cache results under both the submitted PURL and the returned coordinates, preventing false-negative cache poisoning.

Fixes DependencyTrack#7739

Signed-off-by: Pravin Jha <83289698+jha-pravin@users.noreply.github.com>
@codacy-production

codacy-production Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 22 complexity

Metric Results
Complexity 22

View in Codacy

🟢 Coverage ∅ diff coverage · +0.01% coverage variation

Metric Results
Coverage variation ✅ +0.01% coverage variation (-1.00%)
Diff coverage ∅ diff coverage (70.00%)

View coverage diff in Codacy

Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (b9dcc89) Report Missing Report Missing Report Missing
Head commit (6285d1b) 45991 (+51) 40394 (+49) 87.83% (+0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#7757) 0 0 ∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

1 Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Comment on lines +360 to +364
final String scheme = KnownVersioningSchemes.fromPurlType(purl1.getType())
.orElse(
PackageURL.StandardTypes.GOLANG.equals(purl1.getType())
? KnownVersioningSchemes.SCHEME_GOLANG
: null);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GOLANG fallback is dead code, versatile already does exactly this.

Comment on lines +390 to +402
private static boolean areSubpathsEqual(final @Nullable String s1, final @Nullable String s2) {
final String n1 = (s1 == null || s1.isEmpty()) ? null : s1;
final String n2 = (s2 == null || s2.isEmpty()) ? null : s2;
return Objects.equals(n1, n2);
}

private static boolean areQualifiersEqual(
final @Nullable Map<String, String> q1, final @Nullable Map<String, String> q2) {
if ((q1 == null || q1.isEmpty()) && (q2 == null || q2.isEmpty())) {
return true;
}
return Objects.equals(q1, q2);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These do nothing. Subpaths and qualifiers are already dropped in collectAnalyzablePurls.

Comment on lines +384 to +388
private static boolean areNamespacesEqual(final @Nullable String n1, final @Nullable String n2) {
final String s1 = (n1 == null || n1.isEmpty()) ? null : n1;
final String s2 = (n2 == null || n2.isEmpty()) ? null : n2;
return Objects.equals(s1, s2);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Objects.equals over both namespaces would suffice, no need for custom equality logic.

Comment on lines +295 to +297
if (report.coordinates() != null && !report.coordinates().equals(submittedPurl)) {
entriesToCache.put(report.coordinates(), serializedBytes);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caching by both submitted and reported PURLs is purely speculative and adds storage overhead without proven benefit. Only cache by submitted PURL, like before.

Comment on lines +320 to +330
try {
entriesToCache.put(
report.coordinates(), objectMapper.writeValueAsBytes(report.vulnerabilities()));
} catch (IOException e) {
LOGGER.warn(
"Failed to serialize component report for PURL '{}'; Skipping cache",
report.coordinates(),
e);
}
} else {
entriesToCache.put(report.coordinates(), null);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The caching part here should go, see comment above.

- Remove redundant GOLANG scheme fallback in KnownVersioningSchemes.fromPurlType
- Drop unused subpath and qualifier equality checks
- Use standard Objects.equals for namespace comparison
- Cache reports only by submitted PURL

Signed-off-by: Pravin Jha <83289698+jha-pravin@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OSS Index analyzer drops Go findings when response coordinates normalize the version prefix

2 participants