Conversation
Reviewer's GuideIntroduces an authenticated Sequence diagram for the SBOM recommendation report requestsequenceDiagram
participant Client
participant API as RecommendReportEndpoint
participant Service as PurlService
participant DB as ReadOnlyDatabase
Client->>API: POST /v3/recommend/report
API->>DB: begin()
API->>Service: count_sbom_packages(sbom_ids, tx)
alt package limit exceeded
Service-->>API: total package count
API-->>Client: 400 package_limit_exceeded
else within limit
API->>Service: report_for_sboms(sbom_ids, tx)
Service->>DB: fetch_sbom_names(sbom_ids)
Service->>DB: fetch_sbom_purl_rows(sbom_ids)
Service->>Service: find_highest_vendor_patch(pattern, version, candidates)
Service->>DB: fetch vulnerability statuses
Service-->>API: RecommendReportResponse
API-->>Client: 200 aggregated report
end
Flow diagram for recommendation report aggregationflowchart TD
A["SBOM IDs"] --> B[count_sbom_packages]
B --> C{"Total exceeds report_package_limit?"}
C -->|Yes| D["400 package_limit_exceeded"]
C -->|No| E[fetch_sbom_purl_rows]
E --> F[Group by base PURL and version]
F --> G[Pattern-match vendor rebuilds]
G --> H[Select highest vendor patch]
H --> I[Fetch vulnerability and advisory status]
I --> J[Deduplicate package entries and collect found_in]
J --> K[Build impact_summary, sboms, and packages]
K --> L["200 RecommendReportResponse"]
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
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="modules/fundamental/src/purl/service/mod.rs" line_range="913" />
<code_context>
+ sbom_ids: &[Uuid],
+ connection: &C,
+ ) -> Result<RecommendReportResponse, Error> {
+ if sbom_ids.is_empty() || self.recommend_patterns.is_empty() {
+ return Ok(RecommendReportResponse::default());
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** When `recommend_patterns` is empty, `report_for_sboms` returns `RecommendReportResponse::default()` before fetching or populating the requested SBOMs, so the response contains an empty `sboms` list instead of one per requested SBOM.
**Triggers:** When the endpoint is configured without recommendation patterns, such as a caller using the default `Config`.
**Suggested fix:** Populate the per-SBOM zero-count entries before returning, or remove this early return and let the normal empty-result path build the response.
```suggestion
if sbom_ids.is_empty() {
```
</issue_to_address>
### Comment 2
<location path="modules/fundamental/src/purl/service/mod.rs" line_range="1071-1070" />
<code_context>
weakness,
};
-#[derive(Clone, Debug, Default)]
+#[derive(Clone, Debug)]
</code_context>
<issue_to_address>
**issue (broader_impact):** The report emits every vulnerability status returned for the winning vendor PURL, without filtering by `status_slug` or applying the status/remediation rules used by `assemble_recommend_entry`; therefore vulnerabilities with non-impactful statuses such as fixed or not-affected are reported as vulnerabilities for the package.
**Triggers:** When a winning vendor PURL has multiple VEX statuses, including statuses that should not count as applicable vulnerabilities.
**Suggested fix:** Reuse the status filtering logic from `assemble_recommend_entry` when constructing the report's vulnerability list and per-SBOM vulnerability counts.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: modules/fundamental/src/purl/service/mod.rs:913, modules/fundamental/src/purl/service/mod.rs:1070
|
[sdlc-workflow/verify-pr] Re: @sourcery-ai review — Classified as code change request — 2 issues identified; sub-tasks TC-6217 (empty sboms on no patterns) and TC-6218 (vulnerability status filtering) created to address this feedback. |
Verification Report for TC-5999 (commit b99b098)
Overall: FAILBlockers before merge:
This comment was AI-generated by sdlc-workflow/verify-pr v0.13.9. |
| } | ||
|
|
||
| // Fetch vulnerability statuses for all winning versioned PURLs. | ||
| let statuses_by_base = Self::fetch_vulnerability_statuses( |
There was a problem hiding this comment.
something not quite right here (but I am not very familiar with expected output) - if this returns statuses keyed only by base_purl_id even though report contains multiple 'winning' versioned PURLs for that base ? Each package sees a 'union of statuses' from every version and best_by_vuln selects by advisory date rather than the applicable package version. Key statuses by the winning versioned PURL ID as well. Is this what you want ?
There was a problem hiding this comment.
Thanks for the catch, you're right. When two winning PURLs share the same base_purl_id (different upstream versions across the requested SBOMs), statuses_by_base merges their status rows, so best_by_vuln can pick a status entry from the wrong winner if advisory dates differ between them. Fix: key the result by winner_vp_id instead of base_purl_id, select versioned_purl.id in fetch_vulnerability_statuses, and update the lookup to statuses_by_vp.get(&winner.winner_vp_id). Will fix.
| )] | ||
| #[post("/v3/recommend/report")] | ||
| /// Generate an aggregated vendor recommendation report for a set of SBOMs. | ||
| pub async fn recommend_report( |
There was a problem hiding this comment.
this new endpoint returns SBOM names, package PURLs, and SBOM membership but requires only ReadAdvisory perm. Existing PURL/SBOM reads require ReadSbom which is prob needed here as well.
There was a problem hiding this comment.
Agreed, the endpoint returns SBOM names and membership data so ReadSbom should also be required alongside ReadAdvisory. Will add.
Sourcery withdrew this approval because the latest commits introduced blocking findings.
Implements on-demand vendor recommendation report for a list of SBOM IDs. Uses query-time pattern matching (same as /purl/recommend) rather than a dedicated recommendation DB table. Key behaviour: - Counts total packages across all requested SBOMs and returns 400 package_limit_exceeded when the total exceeds TRUSTD_RECOMMEND_REPORT_PACKAGE_LIMIT (default 10 000, configurable via Config.recommend_report_package_limit). - Joins sbom_node_purl_ref → versioned_purl → base_purl to collect upstream PURLs, then runs recommend_patterns regex matching to find the highest vendor patch version for each unique (base, version) pair. - Deduplicates packages that appear in multiple SBOMs; all SBOM IDs appear in found_in on the single package entry. - Fetches VEX statuses for winning vendor PURLs and reports vulnerability IDs and advisory provenance. - Returns RecommendReportResponse with impact_summary, per-sbom breakdown, and deduplicated packages list. Also threads recommend_report_package_limit through Config and the purl endpoint configure function so tests can set an explicit limit without relying on environment variables. Integration tests cover: aggregated data for 2 SBOMs, deduplication with found_in, package limit exceeded → 400, and empty result for unknown packages. Implements TC-5999 Assisted-by: Claude Code
Implements TC-5999 Assisted-by: Claude Code
Previously report_for_sboms returned RecommendReportResponse::default() (empty sboms list) when recommend_patterns was not configured. The correct behaviour is one zero-count RecommendReportSbom entry per requested SBOM, matching the existing winners.is_empty() path. Also regenerates openapi.yaml via cargo xtask precommit — corrects the stale 400 → 413 status code for the package_limit_exceeded response. Adds integration test: POST /v3/recommend/report with empty patterns returns sboms list with one entry at zero counts. Implements TC-6217 Assisted-by: Claude Code
Report's vulnerabilities list previously emitted all VEX status entries regardless of status_slug. Fixed and not_affected statuses are now excluded server-side, since the report returns raw CVE IDs with no status context and callers cannot filter downstream. Only "affected" and "under_investigation" status slugs contribute to the vulnerabilities list and per-SBOM vulnerability_count. Adds integration test verifying that a not_affected CVE is absent from the report's vulnerabilities list even when a vendor patch is found. Implements TC-6218 Assisted-by: Claude Code
Fix 1 (status cross-version contamination): when the same base PURL appears with two different upstream versions (two winner_vp_ids), the batch fetch_vulnerability_statuses call grouped both versions' statuses under the same base_purl_id bucket. best_by_vuln then selected by advisory date across both versions, which could assign a status from the wrong version to a winner. Fix: call fetch_vulnerability_statuses once per winner (keyed by winner_vp_id) to scope version-range matching to each winner's specific versioned PURL. Fix 2 (missing permission): the endpoint returns SBOM names and membership data (found_in SBOM IDs) so ReadSbom must be required alongside ReadAdvisory. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Assisted-by: Claude Code
88b5580 to
fdf880d
Compare
Signed-off-by: Ruben Romero Montes <rromerom@redhat.com>
Summary
POST /v3/recommend/reportendpoint that accepts a list of SBOM IDs and returns an aggregated vendor recommendation reportrecommend_patterns(same approach as the updated/purl/recommendendpoint — norecommendationDB table required)TRUSTD_RECOMMEND_REPORT_PACKAGE_LIMITenv var, default 10 000) returning413 package_limit_exceededwhen exceededfound_inreport_for_sbomsnow returns per-SBOM zero-count entries whenrecommend_patternsis not configured; previously returned an emptysbomslistopenapi.yamlregenerated to correctly show413(not400) forpackage_limit_exceededResponse shape
{ "impact_summary": { "sboms_with_recommendations": N, "addressable_packages": N }, "sboms": [{ "id": UUID, "name": String, "addressable_packages": N, "vulnerability_count": N }], "packages": [{ "purl": String, "recommended_purl": String, "advisory_id": String, "vulnerabilities": [CVE], "found_in": [UUID] }] }Out-of-scope file changes
Two files outside the task's "Files to Modify" list were updated minimally to wire the feature end-to-end:
modules/fundamental/src/endpoints.rs— addedrecommend_report_package_limit: u64toConfigwithDefaultimpl that readsTRUSTD_RECOMMEND_REPORT_PACKAGE_LIMITenv var (enables per-test configuration)server/src/profile/api.rs— added..Default::default()to theConfiginitializer (one line, supplies the new field's default)Test plan
cargo xtask precommit— passesimpact_summary,sboms,packagesfound_in413 package_limit_exceededsbomslist has one entry with zero counts (not empty)Implements TC-5999
🤖 Generated with Claude Code
Summary by Sourcery
Add an aggregated SBOM vendor recommendation report endpoint with configurable request limits and complete per-package and per-SBOM impact reporting.
New Features:
Bug Fixes:
Enhancements:
Tests: