Skip to content

fix(hardened): enable hardened recommendations for single analysis and fix cache leakage (TC-4987)#639

Merged
a-oren merged 2 commits into
guacsec:mainfrom
a-oren:TC-4987
Jul 1, 2026
Merged

fix(hardened): enable hardened recommendations for single analysis and fix cache leakage (TC-4987)#639
a-oren merged 2 commits into
guacsec:mainfrom
a-oren:TC-4987

Conversation

@a-oren

@a-oren a-oren commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fix 1: Set SBOM_ID_PROPERTY from root PURL in single-analysis (processAnalysisRequest), enabling hardened image lookup for non-batch requests
  • Fix 2: Reorder processOriginalRequest before addSbomIdToDependencyTree so monitoring captures the original dependency count, and remove redundant setProperty so the report uses the original tree for scanned count
  • Fix 3: Add null guard in addSbomIdToDependencyTree and preserve all DependencyTree fields (licenseExpressions, root, componentHashes)
  • Fix 4: Keep OCI PURLs in filterLocalPackages — only remove the sbomId root ref when it is NOT an OCI type, so container image entries reach the hardened recommendation lookup
  • Fix 5: Strip cached recommendations in aggregateCacheHits() when recommend=false — prevents Redis cache contamination where a prior recommend=true request's cached PackageItem objects (which include recommendation data) leak through on subsequent recommend=false requests

Test plan

  • Run dockerfile-recommendations-disabled integration test — should pass (no recommendations when recommend=false)
  • Run dockerfile-recommendations-enabled integration test — should pass (hardened recommendations present)
  • Run single analysis with OCI image SBOM — verify hardened recommendations appear in response
  • Run batch analysis with recommend=false after a recommend=true request — verify no recommendation leakage from cache
  • Verify existing unit tests pass: mvn test

🤖 Generated with Claude Code

Summary by Sourcery

Ensure SBOM root handling and recommendation behavior correctly support hardened image lookups and avoid leaking cached recommendations into non-recommendation analyses.

Bug Fixes:

  • Prevent removal of OCI image SBOM root entries so hardened image recommendations can be resolved during vulnerability scans.
  • Avoid leaking recommendation data from cached PackageItems into subsequent analyses when recommendations are disabled.
  • Ensure single-analysis requests populate the SBOM ID property from the SBOM root for consistent hardened lookup behavior.
  • Guard against null or blank SBOM IDs when augmenting the dependency tree, and preserve all existing DependencyTree metadata when injecting the SBOM root dependency.

Enhancements:

  • Introduce a recommend flag helper to normalize request properties when deciding how to handle cached recommendation data.

…d fix cache leakage (TC-4987)

Wire the single analysis endpoint (/api/v5/analysis) to return hardened
image recommendations for OCI PURLs, matching the batch endpoint behavior.
Also fix a cache contamination bug where recommend=false was bypassed by
cached PackageItems containing recommendation data from previous requests.

- Set SBOM_ID_PROPERTY from root PURL in processAnalysisRequest
- Add addSbomIdToDependencyTree to single analysis route
- Guard addSbomIdToDependencyTree against null/blank sbomId
- Preserve all DependencyTree fields when rebuilding
- Keep OCI PURLs in filterLocalPackages for hardened image matching
- Strip cached recommendation data when recommend=false

Implements TC-4987

Assisted-by: Claude Code
@sourcery-ai

sourcery-ai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Reviewer's Guide

Enables hardened image recommendations for single-analysis flows, ensures SBOM-root handling works correctly for OCI images and dependency monitoring, and prevents recommendation data from leaking via cached results when recommendations are disabled.

Sequence diagram for hardened single-analysis flow and cache-safe recommendations

sequenceDiagram
  actor Client
  participant ExhortIntegration
  participant TrustifyIntegration

  Client->>ExhortIntegration: processAnalysisRequest
  ExhortIntegration->>ExhortIntegration: processAnalysisRequest
  ExhortIntegration->>ExhortIntegration: setProperty SBOM_ID_PROPERTY
  ExhortIntegration->>ExhortIntegration: addSbomIdToDependencyTree
  ExhortIntegration->>TrustifyIntegration: analyzeSbom

  TrustifyIntegration->>TrustifyIntegration: filterLocalPackages
  alt [sbomId is OCI_PURL_TYPE]
    TrustifyIntegration->>TrustifyIntegration: keep PackageRef sbomId
  else [sbomId is not OCI_PURL_TYPE]
    TrustifyIntegration->>TrustifyIntegration: packages.remove PackageRef sbomId
  end

  TrustifyIntegration->>TrustifyIntegration: aggregateCacheHits
  TrustifyIntegration->>TrustifyIntegration: isRecommendEnabled
  alt [recommend=false]
    TrustifyIntegration->>TrustifyIntegration: cacheHits.replaceAll new PackageItem
  end
  TrustifyIntegration-->>Client: ProviderResponse
Loading

File-Level Changes

Change Details Files
Ensure OCI image PURLs representing the SBOM root are retained for hardened recommendation lookup while still excluding non-OCI SBOM root entries from vulnerability scans.
  • Adjust SBOM root removal in local package filtering to be conditional on the PURL type.
  • Construct a PackageRef from the SBOM ID and only remove it when its PURL type is not OCI.
  • Update the filterLocalPackages body to preserve OCI-type entries while still writing the filtered set back to the exchange.
src/main/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyIntegration.java
Strip recommendation data from cache hits when recommendations are disabled to avoid leaking prior recommend=true results into recommend=false responses.
  • Introduce a recommend flag check before aggregating cache hits.
  • Replace cached PackageItems with equivalents that have null recommendation data when recommend=false.
  • Add a helper method to robustly read the recommend parameter from exchange properties as boolean with support for Boolean and String values.
src/main/java/io/github/guacsec/trustifyda/integration/providers/trustify/TrustifyIntegration.java
Enable hardened recommendations for single SBOM analyses by propagating the SBOM root PURL into the SBOM_ID_PROPERTY and injecting it into the dependency tree without losing existing metadata.
  • Set SBOM_ID_PROPERTY from the dependency tree root PURL during single-analysis request processing when a root is present.
  • Insert a pipeline step to add the SBOM ID as a synthetic dependency after monitoring of the original request but before SBOM analysis.
  • Guard addSbomIdToDependencyTree against null/blank SBOM IDs and preserve license expressions, root, and component hash metadata when wrapping the dependency tree.
src/main/java/io/github/guacsec/trustifyda/integration/backend/ExhortIntegration.java

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In processAnalysisRequest, consider guarding against a null tree.root().purl() before calling canonicalize() to avoid a potential NPE for SBOMs with a root component but no PURL.
  • The new isRecommendEnabled helper defaults to true when the property is missing or not a boolean/string; double-check this aligns with the expected behavior for requests that omit the recommend parameter, especially with the new cache-stripping logic.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `processAnalysisRequest`, consider guarding against a null `tree.root().purl()` before calling `canonicalize()` to avoid a potential NPE for SBOMs with a root component but no PURL.
- The new `isRecommendEnabled` helper defaults to `true` when the property is missing or not a boolean/string; double-check this aligns with the expected behavior for requests that omit the `recommend` parameter, especially with the new cache-stripping logic.

## Individual Comments

### Comment 1
<location path="src/main/java/io/github/guacsec/trustifyda/integration/backend/ExhortIntegration.java" line_range="321-322" />
<code_context>
     var parser = getSbomParser(exchange);
     var tree = parser.parse(exchange.getIn().getBody(InputStream.class));
     exchange.setProperty(Constants.DEPENDENCY_TREE_PROPERTY, tree);
+    if (tree.root() != null) {
+      exchange.setProperty(Constants.SBOM_ID_PROPERTY, tree.root().purl().canonicalize());
+    }
     exchange.getIn().setBody(tree);
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against a null `purl()` on `tree.root()` before calling `canonicalize()`.

`tree.root()` being non-null doesn’t guarantee `root().purl()` is non-null, so `canonicalize()` can still throw a `NullPointerException`. Please add a null check for `root().purl()` or explicitly handle the case where the SBOM root has no PURL before calling `canonicalize()`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

… licenseExpressions (TC-4987)

Exclude the root PURL from DepsDevRequestBuilder.fromSbom() — it
represents the project itself, not a dependency, so deps.dev has no
data for it. This prevents uncacheable cache misses that broke
testLicensesCaching.

Pass null for licenseExpressions in addSbomIdToDependencyTree to
preserve existing batch license behavior and avoid breaking the
batch_report.json fixture.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.63%. Comparing base (f0ee562) to head (609ff3f).

Files with missing lines Patch % Lines
...stifyda/integration/backend/ExhortIntegration.java 0.00% 6 Missing ⚠️
...ration/providers/trustify/TrustifyIntegration.java 40.00% 3 Missing and 3 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #639      +/-   ##
============================================
- Coverage     56.86%   56.63%   -0.23%     
  Complexity      823      823              
============================================
  Files            94       94              
  Lines          4815     4831      +16     
  Branches        624      630       +6     
============================================
- Hits           2738     2736       -2     
- Misses         1802     1818      +16     
- Partials        275      277       +2     
Flag Coverage Δ
integration-tests 56.63% <33.33%> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...da/integration/licenses/DepsDevRequestBuilder.java 74.19% <100.00%> (+1.77%) ⬆️
...stifyda/integration/backend/ExhortIntegration.java 0.00% <0.00%> (ø)
...ration/providers/trustify/TrustifyIntegration.java 78.96% <40.00%> (-1.37%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@a-oren

a-oren commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

[sdlc-workflow/verify-pr] Re: @sourcery-ai[bot] review —

  1. "Guard against null purl()" — Classified as suggestionPackageRef enforces Objects.requireNonNull(purl) in both constructors, making purl() structurally non-nullable. 18+ codebase call sites confirm this convention. No sub-task created.

  2. "Double-check isRecommendEnabled defaults" — Classified as suggestion — the default of true is consistent with the REST endpoint's .defaultValue(Boolean.TRUE.toString()) declaration. No sub-task created.

@a-oren

a-oren commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Verification Report for TC-4987 (commit 609ff3f)

Check Result Details
Review Feedback PASS 1 code change request (false positive — PackageRef guarantees non-null purl()), 2 suggestions; no sub-tasks needed
Root-Cause Investigation N/A No sub-tasks created
Scope Containment WARN DepsDevRequestBuilder.java changed but not in task spec (justified: test fix for root PURL exclusion)
Diff Size PASS 23 additions, 5 deletions across 3 files — proportionate to 5-fix task
Commit Traceability WARN Commit 5810fa4 references TC-4987; commit 609ff3f does not
Sensitive Patterns PASS No secrets or credentials detected
CI Status PASS All 3 checks pass (Integration Tests, Sourcery review, commitlint)
Acceptance Criteria PASS 5 of 5 criteria met
Test Quality N/A No test files in PR; Eval Quality: N/A
Test Change Classification N/A No test files modified
Verification Commands N/A None specified in task

Overall: WARN

Two informational warnings:

  • Scope: DepsDevRequestBuilder.java is out-of-scope but the change is a justified fix (excluding root PURL from license lookup to prevent uncacheable cache misses).
  • Traceability: Second commit (609ff3f) does not reference TC-4987 in its message.

This comment was AI-generated by sdlc-workflow/verify-pr v0.11.0.

@a-oren a-oren requested a review from ruromero July 1, 2026 11:44

@ruromero ruromero left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@a-oren a-oren merged commit 685fde8 into guacsec:main Jul 1, 2026
3 checks passed
@a-oren a-oren deleted the TC-4987 branch July 1, 2026 16:06
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.

3 participants