feat: stewardship api unmock (CM-1218)#4195
Conversation
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (unstewardedOnly && p.stewardship !== 'unassigned') return false | ||
| if (staleOnly) { | ||
| const lastRelease = MOCK_DETAILS[p.purl]?.general.riskSignals.lastRelease | ||
| if (!lastRelease || new Date(lastRelease) >= staleThreshold) return false |
There was a problem hiding this comment.
Ignored list query filters
Medium Severity
GET /packages still validates and echoes lifecycle and busFactor1Only, but those values are no longer passed into listPackagesForApi, so results stay unfiltered while the response filters object implies they were applied.
Reviewed by Cursor Bugbot for commit 2d2aa85. Configure here.
ulemons
changed the title
There was a problem hiding this comment.
Pull request overview
This PR wires the public v1 Packages/Stewardship API endpoints to the real packages-db (instead of in-memory mocks) by introducing a packages DB connection helper in the backend and a new DAL module (osspckgs/api) that queries package metrics, listings, detail, advisories, and batch stewardship rows.
Changes:
- Add
packagesDbconfig/env wiring (CROWD_PACKAGES_DB_*) and a lazygetPackagesQx()connection helper. - Add new DAL query module
services/libs/data-access-layer/src/osspckgs/api.tsand export it through DAL entrypoints. - Update public API handlers (
/packages,/packages/metrics,/packages/detail,/packages:batch-stewardship) to fetch from the packages DB and return v1 responses (with several v2 fields intentionallynull).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| services/libs/data-access-layer/src/osspckgs/index.ts | Re-export new OSS packages DAL API module. |
| services/libs/data-access-layer/src/osspckgs/api.ts | Introduce SQL queries backing metrics, list, detail, advisories, and batch stewardship lookup. |
| services/libs/data-access-layer/src/index.ts | Export new DAL API module from the package root. |
| backend/src/db/packagesDb.ts | Add lazy, singleton-style packages DB QueryExecutor initializer. |
| backend/src/conf/index.ts | Add PACKAGES_DB_CONFIG configuration entry. |
| backend/src/api/public/v1/packages/listPackages.ts | Replace mock listing logic with DAL-backed pagination and mapping. |
| backend/src/api/public/v1/packages/getPackagesMetrics.ts | Replace mock metrics with DAL-backed metrics query. |
| backend/src/api/public/v1/packages/getPackage.ts | Replace mock detail response with DB-backed package + advisories fetch and response mapping. |
| backend/src/api/public/v1/packages/batchGetStewardship.ts | Replace mock batch stewardship with DB-backed lookup keyed by PURL. |
| backend/config/custom-environment-variables.json | Map packagesDb config to CROWD_PACKAGES_DB_* environment variables. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const qx = await getPackagesQx() | ||
| const { rows, total } = await listPackagesForApi(qx, { | ||
| page, | ||
| pageSize, | ||
| ecosystem, | ||
| staleOnly, | ||
| unstewardedOnly, | ||
| sortBy, | ||
| sortDir, | ||
| }) |
| openVulns: null, | ||
| stewardship: (r.stewardshipStatus ?? 'unassigned') as StewardshipStatus, | ||
| stewards: null, | ||
| })) |
| general: { | ||
| healthScore: null, | ||
| impact: { | ||
| impactScore: pkg.criticalityScore != null ? Math.round(Number(pkg.criticalityScore)) : null, |
There was a problem hiding this comment.
I would suggest to * 100 and then do a Math.round
| ok(res, detail) | ||
| const advisories = await getAdvisoriesByPackageId(qx, pkg.id) | ||
|
|
||
| ok(res, { |
There was a problem hiding this comment.
Why transitiveReach, maintainerBusFactor, resolution, securityContacts (this one we should check with Mouad how he is storing these) are null?
| maintainerBusFactor: null, | ||
| openVulns: null, |
There was a problem hiding this comment.
We could get both of these
| COUNT(*) AS total, | ||
| COUNT(*) FILTER (WHERE has_critical_vulnerability = true) AS critical |
There was a problem hiding this comment.
I don't think we should use `has_critical_vulnerability for this the critical ones. I'm a bit unsure what Nuno meant by this, but for now we could use the ones where Health is critical. Let's add a todo here to confirm this with product, and we can update later.
… filter Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
ulemons
force-pushed
the
feat/backfill-stewardship-script
branch
from
399b9ba to
a739c9f
Compare
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
ulemons
force-pushed
the
feat/stewardship-api-unmock
branch
from
2d2aa85 to
3066140
Compare
| impact: r.criticalityScore != null ? Math.round(Number(r.criticalityScore) * 100) : null, | ||
| lifecycle: null, | ||
| maintainerBusFactor: null, | ||
| openVulns: r.openVulns, |
There was a problem hiding this comment.
List openVulns wrong response shape
High Severity
GET /packages now puts a plain integer in openVulns, but the public contract and OpenVulns type expect an object with low, medium, high, and critical counts (or null). Clients that read severity fields from the list response will get wrong or missing data.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3066140. Configure here.
| SELECT | ||
| COUNT(*) AS total, | ||
| -- TODO: confirm with product whether "critical" here means health=critical, not has_critical_vulnerability | ||
| COUNT(*) FILTER (WHERE has_critical_vulnerability = true) AS critical |
There was a problem hiding this comment.
Metrics critical count wrong criterion
Medium Severity
getPackageMetrics sets criticalPackages using has_critical_vulnerability, while product review on this PR indicated that metric should reflect critical health (with confirmation pending), not the vulnerability flag. Dashboard totals can disagree with intended stewardship semantics.
Reviewed by Cursor Bugbot for commit 3066140. Configure here.
| WHEN ar.fixed_version IS NULL AND ar.last_affected IS NULL THEN FALSE | ||
| WHEN ar.fixed_version IS NOT NULL AND p.latest_version >= ar.fixed_version THEN TRUE | ||
| WHEN ar.fixed_version IS NOT NULL THEN FALSE | ||
| WHEN ar.last_affected IS NOT NULL AND p.latest_version > ar.last_affected THEN TRUE |
There was a problem hiding this comment.
Advisory resolution uses string compare
Medium Severity
getAdvisoriesByPackageId marks advisories patched or open by comparing latest_version to range bounds with SQL >= and >. That is lexicographic, not semver or Maven ordering, so patched vs open can be wrong for typical version strings.
Reviewed by Cursor Bugbot for commit 3066140. Configure here.
| pkg.downloadsLast30d != null ? parseInt(pkg.downloadsLast30d, 10) : null, | ||
| dependentPackages: pkg.dependentPackagesCount ?? null, | ||
| dependentRepos: pkg.dependentReposCount ?? null, | ||
| transitiveReach: pkg.transitiveReach, |
There was a problem hiding this comment.
transitiveReach wrong API format
Medium Severity
Package detail returns transitiveReach as a numeric percentile rank from SQL, while the public schema and previous mock responses use a human-readable string such as Top 0.4%. Consumers expecting the documented string format will misinterpret or fail to render the field.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3066140. Configure here.
ulemons
force-pushed
the
feat/backfill-stewardship-script
branch
from
a739c9f to
4e73636
Compare
| }, | ||
| provenance: { | ||
| repositoryMapping: { | ||
| declaredRepo: pkg.repoUrl ?? pkg.repositoryUrl ?? pkg.declaredRepositoryUrl ?? null, |
There was a problem hiding this comment.
Declared repo uses mapped URL
Medium Severity
provenance.repositoryMapping.declaredRepo prefers repoUrl from the best-confidence repo join before declaredRepositoryUrl, so the field can expose an inferred mapping instead of the package’s declared repository.
Reviewed by Cursor Bugbot for commit 8a2b724. Configure here.
| health: null, | ||
| impact: r.criticalityScore != null ? Math.round(Number(r.criticalityScore) * 100) : null, | ||
| lifecycle: null, | ||
| maintainerBusFactor: null, | ||
| openVulns: r.openVulns, | ||
| stewardship: (r.stewardshipStatus ?? 'unassigned') as StewardshipStatus, |
| pkg.downloadsLast30d != null ? parseInt(pkg.downloadsLast30d, 10) : null, | ||
| dependentPackages: pkg.dependentPackagesCount ?? null, | ||
| dependentRepos: pkg.dependentReposCount ?? null, | ||
| transitiveReach: pkg.transitiveReach, | ||
| }, |
| p.latest_release_at AS "latestReleaseAt", | ||
| s.status AS "stewardshipStatus", | ||
| (SELECT COUNT(*)::int FROM advisory_packages ap WHERE ap.package_id = p.id) AS "openVulns", | ||
| COUNT(*) OVER() AS total | ||
| FROM packages p | ||
| LEFT JOIN stewardships s ON s.package_id = p.id |
| -- percentile rank within ecosystem (0=least, 1=most transitive reach) | ||
| ( | ||
| SELECT r.prank | ||
| FROM ( | ||
| SELECT purl, PERCENT_RANK() OVER (PARTITION BY ecosystem ORDER BY transitive_dependent_count ASC NULLS FIRST) AS prank | ||
| FROM packages | ||
| WHERE ecosystem = p.ecosystem | ||
| ) r | ||
| WHERE r.purl = p.purl | ||
| ) AS "transitiveReach" |
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 8 total unresolved issues (including 6 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dc97258. Configure here.
| LEFT JOIN stewardships s ON s.package_id = p.id | ||
| LEFT JOIN LATERAL ( | ||
| SELECT COUNT(*)::int AS cnt FROM advisory_packages WHERE package_id = p.id | ||
| ) ap_counts ON true |
There was a problem hiding this comment.
openVulns counts all advisories
Medium Severity
The list query labels the lateral count as openVulns, but it counts every advisory_packages row for the package with no check for patched or open resolution. Sorting and display by openVulns therefore reflect total linked advisories, not open vulnerabilities.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit dc97258. Configure here.
| LIMIT 1 | ||
| ) pr ON true | ||
| LEFT JOIN repos r ON r.id = pr.repo_id | ||
| WHERE p.purl = $(purl) |
There was a problem hiding this comment.
Detail omits critical package scope
Medium Severity
Package detail loads any row by purl, while the list API restricts to is_critical = true. Non-critical packages in the same table can return full detail even though they never appear in the stewardship list.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit dc97258. Configure here.
| health: null, | ||
| impact: r.criticalityScore != null ? Math.round(Number(r.criticalityScore) * 100) : null, | ||
| lifecycle: null, | ||
| maintainerBusFactor: null, | ||
| openVulns: r.openVulns, | ||
| stewardship: (r.stewardshipStatus ?? 'unassigned') as StewardshipStatus, |
| const querySchema = z.object({ | ||
| page: z.coerce.number().int().min(1).default(1), | ||
| pageSize: z.coerce.number().int().min(1).max(MAX_PAGE_SIZE).default(DEFAULT_PAGE_SIZE), | ||
| ecosystem: z.string().trim().optional(), | ||
| lifecycle: z.enum(lifecycleValues).optional(), | ||
| busFactor1Only: booleanQueryParam, | ||
| lifecycle: z.enum(lifecycleValues).optional(), // TODO: filter not yet implemented in DAL | ||
| busFactor1Only: booleanQueryParam, // TODO: filter not yet implemented in DAL | ||
| staleOnly: booleanQueryParam, | ||
| unstewardedOnly: booleanQueryParam, | ||
| sortBy: z.enum(['name', 'health', 'impact', 'openVulns']).default('name'), | ||
| sortDir: z.enum(['asc', 'desc']).default('asc'), | ||
| }) |
| }, | ||
| sort: { by: sortBy, dir: sortDir }, | ||
| sort: { by: effectiveSortBy, dir: sortDir }, | ||
| packages, |
| SELECT | ||
| a.osv_id AS "osvId", | ||
| LOWER(a.severity) AS severity, | ||
| CASE |
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Summary
Removes all mock implementations from the 4 packages public API endpoints and replaces them with real queries against the packages DB. Also lands the stewardship schema migration and a backfill script to seed one unassigned stewardship row per critical package.
Changes
Type of change
JIRA ticket
ticket
Note
Medium Risk
Public API responses now depend on packages DB availability and real data semantics; advisory patched/open logic uses lexicographic version comparison, which can misclassify semver ordering.
Overview
Wires the public packages API to the packages database instead of in-memory mocks, via
CROWD_PACKAGES_DB_*config and a lazygetPackagesQx()connection helper.Four endpoints now use new DAL queries in
osspckgs/api.ts: metrics counts critical packages, list supports pagination/filters (ecosystem, stale, unstewarded) and DB-side sort, batch stewardship resolves by PURL, and package detail assembles impact, repo/scorecard fields, stewardship status, and advisories with open/patched resolution. Response shape is preserved but several fields are intentionallynulluntil later work (health, lifecycle, stewards, transitive reach); list still acceptslifecycle/busFactor1Onlyquery params but those filters are not applied in the DAL yet;sortBy=healthfalls back to name.getPackagevalidatespkg:PURLs with Zod instead of a manualBadRequestError.Reviewed by Cursor Bugbot for commit eefb9ed. Bugbot is set up for automated code reviews on this repo. Configure here.