Closed dwertent closed 6 months ago
PR Description updated to latest commit (https://github.com/kubescape/kubevuln/commit/1995f27f8ca71bd708e3151f29a255e093d73be0)
⏱️ Estimated effort to review [1-5] | 2, because the changes are straightforward and localized to specific functionalities, mainly focusing on version handling and SBOM validation. The logic is not complex, but understanding the context of versioning and SBOM handling is necessary. |
🧪 Relevant tests | No |
🔍 Possible issues | Version Dependency: Hardcoding "-hotfix" to the version might cause issues in future upgrades or version comparisons. Consider a more dynamic approach to version management. |
Error Handling: The introduction of `ErrOutdatedSBOM` without showing its handling in the PR might lead to uncaught errors or improper error handling in the application flow. | |
🔒 Security concerns | No |
Category | Suggestions |
Best practice |
Handle potential errors when fetching the package version.___ **Consider handling the scenario wheretools.PackageVersion might return an error alongside the version string. This ensures that any issues encountered while fetching the package version are properly handled, rather than potentially misinterpreting them as "unknown" or an empty string.** [adapters/v1/grype.go [199]](https://github.com/kubescape/kubevuln/pull/223/files#diff-665c620467429b3c5376c54c77dfbc3fd78f7f5cda60b1c53cc36fd0b1a9f39eR199-R199) ```diff -v := tools.PackageVersion("github.com/anchore/grype") +v, err := tools.PackageVersion("github.com/anchore/grype") +if err != nil { + // Handle error, possibly logging it or returning an error version string +} ``` |
Add logging for better traceability when validating SBOMs.___ **In theGetSBOMp function, after validating the SBOM manifest, consider adding a log statement that includes the SBOM name and version. This will provide better traceability and debugging capabilities, especially when dealing with multiple SBOMs.** [repositories/apiserver.go [848]](https://github.com/kubescape/kubevuln/pull/223/files#diff-dcc3484e8a0759bddcff34049e6114ed941f40176d0f3c1e4a84841dfabf6403R848-R848) ```diff if err := validateSBOMp(manifest, sbomCreatorVersion); err != nil { + // Log the SBOM name and version for better traceability + logger.L().Info("Validating SBOM", helpers.String("name", name), helpers.String("version", sbomCreatorVersion)) +} ``` | |
Enhancement |
Ensure the new error for outdated SBOMs is properly handled.___ **Since you've added a new error for outdated SBOMs, ensure that this error is properlyhandled wherever SBOM versions are checked. This might involve adding checks in functions that load or process SBOMs to throw this error when an outdated SBOM is detected.** [core/domain/scan.go [19]](https://github.com/kubescape/kubevuln/pull/223/files#diff-72a9841d2f895677acbce1929094dc617c62a91856dacdfe90f3ecb5c8a4a1d0R19-R19) ```diff -ErrOutdatedSBOM = errors.New("SBOM is outdated") +// Ensure to handle ErrOutdatedSBOM in SBOM processing functions ``` |
Implement the check for SBOMCreatorVersion to ensure compatibility.___ **For the TODO comment about checkingSBOMCreatorVersion , it's crucial to implement this check to ensure compatibility between SBOMs and the scanner. Consider prioritizing this implementation to avoid potential issues with version mismatches.** [repositories/apiserver.go [90]](https://github.com/kubescape/kubevuln/pull/223/files#diff-dcc3484e8a0759bddcff34049e6114ed941f40176d0f3c1e4a84841dfabf6403R90-R90) ```diff -// TODO: also check SBOMCreatorVersion ? - we should, but we don't have the version in the manifest +// Implement check for SBOMCreatorVersion to ensure compatibility ``` | |
Improve error messaging for SBOM version mismatches.___ **When validating the SBOM version invalidateSBOMp , consider adding a more descriptive error message that includes the expected and actual versions. This will aid in debugging and understanding version mismatch issues more clearly.** [repositories/apiserver.go [823-824]](https://github.com/kubescape/kubevuln/pull/223/files#diff-dcc3484e8a0759bddcff34049e6114ed941f40176d0f3c1e4a84841dfabf6403R823-R824) ```diff if manifest.Spec.Metadata.Tool.Version != sbomCreatorVersion { - return domain.ErrOutdatedSBOM + return fmt.Errorf("%w: expected %s, got %s", domain.ErrOutdatedSBOM, sbomCreatorVersion, manifest.Spec.Metadata.Tool.Version) } ``` |
Summary:
User description
Overview
After testing with the node-agent, we need to ignore sbomp with older version
Type
enhancement, bug_fix
Description
adapters/v1/grype.go
to ensure SBOMs are re-created with the hotfix version.ErrOutdatedSBOM
incore/domain/scan.go
for handling outdated SBOMs.core/services/scan.go
.repositories/apiserver.go
.Changes walkthrough
grype.go
Append Hotfix Version to Grype Version
adapters/v1/grype.go
version.
scan.go
Introduce Error for Outdated SBOMs
core/domain/scan.go
ErrOutdatedSBOM
for handling outdated SBOMs.scan.go
Cleanup and Ensure CVE Manifest Creation
core/services/scan.go
apiserver.go
Discard Outdated CVE Manifests and SBOMs Based on Version
repositories/apiserver.go
manifests.
validateSBOMp
to discard outdated SBOMs.GetSBOMp
to passsbomCreatorVersion
tovalidateSBOMp
.