Support for snyk's full, partial and no match response - #6835
Conversation
Signed-off-by: Meha Bhargava <meha.bhargava2@gmail.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 115 |
🟢 Coverage 86.60% diff coverage · +0.00% coverage variation
Metric Results Coverage variation ✅ +0.00% coverage variation (-1.00%) Diff coverage ✅ 86.60% diff coverage (70.00%) Coverage variation details
Coverable lines Covered lines Coverage Common ancestor commit (888cb05) 42943 37295 86.85% Head commit (90fcd3f) 43120 (+177) 37449 (+154) 86.85% (+0.00%) 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 (#6835) 209 181 86.60% 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%
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.
| */ | ||
| static boolean requiresChecksumMeta(PackageURL purl, boolean checksumMatchingEnabled) { | ||
| return checksumMatchingEnabled | ||
| && MAVEN.equals(purl.getType()) |
There was a problem hiding this comment.
Is the limitation to Maven a vendor restriction or an active choice by you?
| /** | ||
| * Compact cache encoding for Snyk analyzer results. | ||
| * <p> | ||
| * Empty or untrusted outcomes use single-byte sentinels or {@code null} so portfolio-scale | ||
| * cache reads avoid JSON deserialization for the common negative-cache case. | ||
| * | ||
| * @since 5.1.0 | ||
| */ | ||
| final class SnykCacheCodec { |
There was a problem hiding this comment.
I don't quite follow why this was needed. The old code already cached null (i.e. no serialization involved) for negative hits:
And null values were simply skipped, there's no deserialization:
| if (isChecksumQualifiedRequestPurl(requestPurl)) { | ||
| if (cached.matchType() != SnykMatchType.FULL) { | ||
| // PARTIAL / NONE on checksum-qualified key: skip findings, do not re-fetch. | ||
| return; | ||
| } | ||
| // FULL checksum-qualified match: attach cached issues when present (or negative cache when empty). | ||
| attachCachedIssuesIfPresent(requestPurl, cached, issuesByPurl); | ||
| return; | ||
| } | ||
| // Coordinates-only keys are always cached as FULL; attach issues when present. | ||
| attachCachedIssuesIfPresent(requestPurl, cached, issuesByPurl); | ||
| } | ||
|
|
||
| private void attachCachedIssuesIfPresent( | ||
| String requestPurl, | ||
| SnykCachedPurlResult cached, | ||
| Map<String, List<SnykIssue>> issuesByPurl) { | ||
| if (cached.issues() != null && !cached.issues().isEmpty()) { | ||
| issuesByPurl.put(requestPurl, cached.issues()); | ||
| } | ||
| // Empty issues is a cached negative result (shared across projects until TTL). | ||
| } |
There was a problem hiding this comment.
Handling for match types PARTIAL, NONE, and FULL-with-empty-issues is behaviorally the same. I think for all those cases we should just cache null, which removes the SnykCacheCodec overhead, slightly reduces storage cost, and makes the code simpler.
| assertThat(analyzer.analyze(bom).getVulnerabilitiesList()).isEmpty(); | ||
|
|
||
| verify(2, postRequestedFor(anyUrl())); | ||
| } |
There was a problem hiding this comment.
Should this not cause a negative cache? If a PURL can't be analyzed for a permanent reason (e.g. due to Unsupported Ecosystem as in the test file), it won't be analyzable on the next run.
| // Issue PURL may include qualifiers; match on coordinates. | ||
| for (final var entry : issuesByIssuePurl.entrySet()) { | ||
| final String coords = coordinatesLower(entry.getKey()); | ||
| if (requestPurl.equals(coords) && bomRefsByPurl.containsKey(requestPurl)) { | ||
| issues.addAll(entry.getValue()); | ||
| } | ||
| } |
There was a problem hiding this comment.
This could be a source of false positives, and also somewhat defeats the purpose of the checksum matching, since the checksums here are also just PURL qualifiers which would be discarded.
| "checksumMatchingEnabled": { | ||
| "type": "boolean", | ||
| "title": "Checksum Matching Enabled", | ||
| "description": "When enabled, send checksum qualifiers from Maven component PURLs to Snyk and interpret meta.packages match results. Only pkg:maven PURLs with a checksum qualifier use checksum matching; all others use coordinates-only matching.", |
There was a problem hiding this comment.
There are a few preconditions to make this feature work to begin with, which should be documented. For example, from what I understand, this practically only works when you generate your SBOMs with Snyk's tooling, as no other generator emits checksum PURL qualifiers.
| */ | ||
| static boolean requiresChecksumMeta(PackageURL purl, boolean checksumMatchingEnabled) { | ||
| return checksumMatchingEnabled | ||
| && MAVEN.equals(purl.getType()) |
There was a problem hiding this comment.
| && MAVEN.equals(purl.getType()) | |
| && PackageURL.StandardTypes.MAVEN.equals(purl.getType()) |
| try { | ||
| return SnykPurlUtil.requiresChecksumMeta(new PackageURL(requestPurl), true); | ||
| } catch (MalformedPackageURLException e) { | ||
| return requestPurl.contains("checksum="); |
There was a problem hiding this comment.
requestPurl always comes from SnykPurlUtil.toSnykRequestPurl and should never be invalid. The fallback is dangerous because it could match on arbitrary prefixes of checksum like foo_checksum=. Either return false here or propagate the exception.
Description
Adds opt-in Snyk checksum-qualified PURL matching for Maven components, and compact result-cache encoding so large portfolio scans avoid Jackson deserialization for the common “no findings” path.
When checksumMatchingEnabled is true, Maven PURLs with a checksum qualifier are sent to Snyk as full canonical PURLs and evaluated via meta.packages match types (full / partial / none). Coordinates-only and flag-off behavior is unchanged.
Addressed Issue
#6204
Additional Details
Why
Snyk’s batch packages/issues API (2025-11-05) accepts checksum-qualified PURLs (e.g. from snyk sbom --include-provenance) and returns match quality in meta.packages. Previously Dependency-Track stripped PURL qualifiers, ignored meta, and could not distinguish a trustworthy empty result from an untrusted partial/none match.
Runtime behaviour
Config: checksumMatchingEnabled(default false) on the Snyk vuln-analyzer extension.
Coordinates-only (flag off, or no checksum on the sent PURL):
Checksum-qualified (flag on + Maven + checksum qualifier):
No apiserver / BOM prep changes — checksums must already be present on component PURLs.
Cache design (portfolio scale)
Keys are shared by request PURL across projects (scales with unique PURLs, not project count).
Cache read branches on request key shape:
SnykCacheCodec stores compact values when there are no findings:
Legacy formats remain readable (null, SnykIssue[] JSON, structured SnykCachedPurlResult JSON).
Checklist
docs/adr/