Conversation
…ovate-bot-feature
…ard into renovate-bot-feature
…ts (model/DTO/transformer/migrations/aggregation/query wiring) Current problem: open_dependency_vulns is populated correctly, but fixable_* and cve_purl_fixable_* are still written as 0
There was a problem hiding this comment.
Pull request overview
This PR updates statistics/risk distribution data to include “fixable” vulnerability counts, and adds an endpoint to return a recommended fixed version for a dependency.
Changes:
- Extend statistics distribution DTO/model and risk history queries to include fixable counts (and CVE+purl fixable counts).
- Add DB migration to store fixable distribution columns in
artifact_risk_history. - Add a dependency recommendation endpoint and repository query to fetch a fixed-version hint; regenerate/update mockery mocks.
Reviewed changes
Copilot reviewed 87 out of 203 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mocks/SBOMScanner.go | Adds mock for SBOM scanner interface used in tests. |
| mocks/Resolver.go | Adds generic resolver mock for fixed-version resolution logic. |
| mocks/RequestSigner.go | Adds mock for request signing. |
| mocks/Repository.go | Adds generic repository mock for data access tests. |
| mocks/ReportingDescriptorReference.go | Updates generated mockery output/header and mock structure. |
| mocks/ReleaseService.go | Adds mock for release service. |
| mocks/RBACProvider.go | Adds mock for RBAC provider. |
| mocks/PublicClient.go | Adds mock for Ory public client. |
| mocks/PubSubMessage.go | Adds mock for pub/sub message. |
| mocks/PubSubBroker.go | Adds mock for pub/sub broker. |
| mocks/ProjectService.go | Adds mock for project service. |
| mocks/ProjectRiskHistoryRepository.go | Adds mock for project risk history repository. |
| mocks/PersonalAccessTokenService.go | Adds mock for PAT service. |
| mocks/OrgService.go | Adds mock for org service. |
| mocks/OpenSourceInsightService.go | Adds mock for OSI service. |
| mocks/ModelWriter.go | Adds generic model-writer mock. |
| mocks/ModelReader.go | Adds generic model-reader mock. |
| mocks/MiddlewareFunc.go | Adds mock for middleware wrapper function. |
| mocks/MaliciousPackageChecker.go | Adds mock for malicious package checker. |
| mocks/LicenseRiskService.go | Adds mock for license risk service. |
| mocks/LeaderElector.go | Adds mock for leader election helper. |
| mocks/JiraIntegrationRepository.go | Adds mock for Jira integration repository. |
| mocks/InvitationRepository.go | Adds mock for invitation repository. |
| mocks/IntegrationAggregate.go | Adds mock for integration aggregation layer. |
| mocks/InTotoVerifierService.go | Adds mock for in-toto verification service. |
| mocks/GraphComponent.go | Adds mock for SBOM graph component abstraction. |
| mocks/GitlabIntegrationRepository.go | Adds mock for GitLab integration repository. |
| mocks/GitlabClientFactory.go | Adds mock for GitLab client factory. |
| mocks/GithubClientFacade.go | Adds mock for GitHub client facade. |
| mocks/GithubAppInstallationRepository.go | Adds mock for GitHub App installation repository. |
| mocks/GitLabOauth2TokenRepository.go | Adds mock for GitLab OAuth token repository. |
| mocks/FixedVersionResolver.go | Adds mock for fixed-version resolver. |
| mocks/FirstPartyVulnService.go | Adds mock for first-party vuln service. |
| mocks/FireAndForgetSynchronizer.go | Adds mock for async “fire and forget” helper. |
| mocks/ExternalUserRepository.go | Adds mock for external user repository. |
| mocks/ExternalReferenceRepository.go | Adds mock for external reference repository. |
| mocks/ExternalPropertyFileReference.go | Updates generated mockery output/header and mock structure. |
| mocks/ExternalEntityProviderService.go | Adds mock for external entity provider service. |
| mocks/ExploitRepository.go | Adds mock for exploit repository. |
| mocks/DependencyVulnService.go | Adds mock for dependency vuln service. |
| mocks/DaemonRunner.go | Adds mock for daemon runner orchestration. |
| mocks/CweRepository.go | Adds mock for CWE repository. |
| mocks/ConfigService.go | Adds mock for config service. |
| mocks/ConfigRepository.go | Adds mock for config repository. |
| mocks/ComponentService.go | Adds mock for component service. |
| mocks/ComponentProjectRepository.go | Adds mock for component project repository. |
| mocks/CSAFService.go | Adds mock for CSAF service. |
| mocks/BatchModelWriter.go | Adds mock for batch model writer. |
| mocks/AuthSession.go | Adds mock for auth session wrapper. |
| mocks/AssetVersionService.go | Adds mock for asset version service. |
| mocks/AssetService.go | Adds mock for asset service. |
| mocks/ArtifactService.go | Adds mock for artifact service. |
| mocks/ArtifactRiskHistoryRepository.go | Adds mock for artifact risk history repository. |
| mocks/AffectedComponentRepository.go | Adds mock for affected component repository. |
| mocks/AdminClient.go | Adds mock for Ory admin client. |
| dtos/statistics_dto.go | Extends statistics distribution DTO with fixable counts. |
| dtos/recommendation_dto.go | Adds DTO for dependency update recommendation response. |
| database/repositories/statistics_repository.go | Adjusts dependency vuln time-travel queries (select + artifact filter fix). |
| database/repositories/dependency_vuln_repository.go | Adds query to fetch direct dependency fixed-version hint by package name. |
| database/repositories/artifact_risk_history_repository.go | Extends risk history SQL projections/aggregations with fixable columns. |
| database/models/statistic_model.go | Extends statistics distribution model with fixable counts. |
| database/migrations/20260402110000_add_fixable_distribution_to_artifact_risk_history.up.sql | Adds fixable distribution columns to artifact_risk_history. |
| database/migrations/20260402110000_add_fixable_distribution_to_artifact_risk_history.down.sql | Drops fixable distribution columns. |
| database/migrations/20260304170435_recover_missing_migration.up.sql | Adds empty recovery migration placeholder. |
| database/migrations/20260304170435_recover_missing_migration.down.sql | Adds empty recovery migration placeholder (down). |
| controllers/dependency_vuln_controller.go | Adds recommendation endpoint and PURL version parsing helper. |
| cmd/devguard-cli/commands/daemon.go | Adds fixedversion.Module to CLI daemon pipeline DI graph. |
| .mockery.yaml | Removes mockery configuration file. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| currentValue := ctx.QueryParam("packageValue") | ||
| if currentValue == "" { | ||
| currentValue = ctx.QueryParam("currentValue") | ||
| } | ||
| if currentValue == "" { | ||
| return echo.NewHTTPError(400, "missing packageValue or currentValue") | ||
| } | ||
|
|
There was a problem hiding this comment.
currentValue is parsed and validated but never used afterwards, which will cause a Go compile error (declared and not used). Either use currentValue in the recommendation logic (e.g., compare/validate the returned recommendation against the current version) or remove this block entirely.
| currentValue := ctx.QueryParam("packageValue") | |
| if currentValue == "" { | |
| currentValue = ctx.QueryParam("currentValue") | |
| } | |
| if currentValue == "" { | |
| return echo.NewHTTPError(400, "missing packageValue or currentValue") | |
| } |
| err = r.GetDB(ctx, tx).Model(&models.DependencyVuln{}).Select("dependency_vulns.*").Preload("CVE").Preload("Events", func(db *gorm.DB) *gorm.DB { | ||
| return db.Where("created_at <= ?", time).Order("created_at ASC") | ||
| }). | ||
| Joins("JOIN artifact_dependency_vulns adv ON adv.dependency_vuln_id = dependency_vulns.id"). | ||
| Where("adv.artifact_asset_version_name = ?", *assetVersionName).Where("adv.artifact_asset_id = ?", assetID).Where("adv.artifact_artifact_name = ?", artifactName).Where("created_at <= ?", time). | ||
| Where("adv.artifact_asset_version_name = ?", *assetVersionName).Where("adv.artifact_asset_id = ?", assetID).Where("adv.artifact_artifact_name = ?", *artifactName).Where("created_at <= ?", time). | ||
| Find(&dependencyVulns).Error |
There was a problem hiding this comment.
This branch dereferences *assetVersionName but the branch condition only checks artifactName != nil. If callers pass an artifact name without an asset version name, this will panic. Consider either (1) requiring assetVersionName != nil in this branch and returning an error otherwise, or (2) adjusting the query to not dereference assetVersionName when it is nil.
| err = r.GetDB(ctx, tx).Model(&models.DependencyVuln{}).Preload("CVE").Preload("Events", func(db *gorm.DB) *gorm.DB { | ||
| err = r.GetDB(ctx, tx).Model(&models.DependencyVuln{}).Select("dependency_vulns.*").Preload("CVE").Preload("Events", func(db *gorm.DB) *gorm.DB { | ||
| return db.Where("created_at <= ?", time).Order("created_at ASC") | ||
| }).Where("adv.artifact_asset_id = ?", assetID).Where("adv.artifact_artifact_name = ?", artifactName).Where("created_at <= ?", time). |
There was a problem hiding this comment.
The query filters on adv.* columns but does not join the artifact_dependency_vulns adv table in this branch. This will produce an SQL error (unknown table/alias adv) at runtime. Add the same JOIN used in the other branches (or remove the adv.* predicates if they’re not intended here).
| }).Where("adv.artifact_asset_id = ?", assetID).Where("adv.artifact_artifact_name = ?", artifactName).Where("created_at <= ?", time). | |
| }). | |
| Joins("JOIN artifact_dependency_vulns adv ON adv.dependency_vuln_id = dependency_vulns.id"). | |
| Where("adv.artifact_asset_id = ?", assetID).Where("adv.artifact_artifact_name = ?", artifactName).Where("created_at <= ?", time). |
| err := repository.GetDB(ctx, tx). | ||
| WithContext(ctx). | ||
| Table("dependency_vulns"). | ||
| Where("direct_dependency_fixed_version IS NOT NULL AND direct_dependency_fixed_version != ''"). | ||
| Where("EXISTS (SELECT 1 FROM jsonb_array_elements(vulnerability_path) AS purl WHERE purl::text LIKE '%/' || ? || '@%')", packageName). | ||
| Select("direct_dependency_fixed_version"). | ||
| Order("last_detected DESC"). | ||
| Limit(1). | ||
| Scan(&directDependencyFixedVersion).Error |
There was a problem hiding this comment.
This query expands vulnerability_path with jsonb_array_elements(...) and applies a leading-wildcard LIKE predicate, which is likely to be slow on large datasets and hard to index. If this endpoint is latency-sensitive, consider persisting a searchable representation (e.g., extracting direct dependency package name into a dedicated column or using a GIN-indexable jsonb containment query) to avoid per-row JSON expansion + wildcard matching.
958516a to
71f57d4
Compare
71f57d4 to
5d16bda
Compare
No description provided.