feat: add --vuln-severity-source flag
#8269
Conversation
pkg/vulnerability/vulnerability.go
Outdated
| } | ||
| } | ||
| if len(severitySrc) != 1 || severitySrc[0] != "auto" { | ||
| log.Warn("No severity found in specified sources", |
There was a problem hiding this comment.
I know these logs may be noise, but warnings should be rare occurrences and we should draw the user's attention to each vulnerability.
But we can also show the warning once and use debug logs for each vulnerability:
2025-01-22T15:47:14+06:00 WARN For one or more vulnerabilities, the severity level is not found in the specified sources
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2016-2781" severity sources="alpine,alma"
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2022-3219" severity sources="alpine,alma"
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2016-20013" severity sources="alpine,alma"
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2016-20013" severity sources="alpine,alma"
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2024-41996" severity sources="alpine,alma"
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2024-56433" severity sources="alpine,alma"
2025-01-22T15:47:14+06:00 DEBUG No severity found in specified sources vulnID="CVE-2024-56433" severity sources="alpine,alma"
| @@ -105,6 +105,7 @@ trivy image [flags] IMAGE_NAME | |||
| --secret-config string specify a path to config file for secret scanning (default "trivy-secret.yaml") | |||
| --server string server address in client mode | |||
| -s, --severity strings severities of security issues to be displayed (UNKNOWN,LOW,MEDIUM,HIGH,CRITICAL) (default [UNKNOWN,LOW,MEDIUM,HIGH,CRITICAL]) | |||
| --severity-src strings order of data sources for selecting vulnerability severity level (nvd,redhat,redhat-oval,debian,ubuntu,alpine,amazon,oracle-oval,suse-cvrf,photon,arch-linux,alma,rocky,cbl-mariner,azure,ruby-advisory-db,php-security-advisories,nodejs-security-wg,ghsa,glad,aqua,osv,k8s,wolfi,chainguard,bitnami,govulndb,auto) (default [auto]) | |||
There was a problem hiding this comment.
This line is very long, but I wanted to immediately stop Trivy for the incorrect sourceID.
|
@knqyf263 I think this PR is ready for review. If you don't have comments about trivy-db changes - we can merge aquasecurity/trivy-db#485. |
pkg/flag/vulnerability_flags.go
Outdated
| @@ -30,19 +32,30 @@ var ( | |||
| ConfigName: "vulnerability.skip-vex-repo-update", | |||
| Usage: `[EXPERIMENTAL] Skip VEX Repository update`, | |||
| } | |||
| SeveritySrcFlag = Flag[[]string]{ | |||
| Name: "severity-src", | |||
There was a problem hiding this comment.
Actually, we have several types of findings, like misconfigurations, while this flag is dedicated to vulnerabilities. Is --vuln-severity-src better? Do you have any other ideas?
There was a problem hiding this comment.
for non-vuln findings only one source is available.
So I think you are right. We should use the vuln prefix to avoid confusion for the user.
There was a problem hiding this comment.
changed in 0f42477
There was a problem hiding this comment.
We also need to update the PR title and description.
There was a problem hiding this comment.
I'm kind of absent-minded today...
thanks! I updated!
pkg/flag/vulnerability_flags.go
Outdated
| } | ||
|
|
||
| type VulnerabilityOptions struct { | ||
| IgnoreStatuses []dbTypes.Status | ||
| VEXSources []vex.Source | ||
| SkipVEXRepoUpdate bool | ||
| SeveritySrc []string |
There was a problem hiding this comment.
| SeveritySrc []string | |
| SeveritySources []dbTypes.SourceID |
pkg/flag/vulnerability_flags.go
Outdated
| @@ -101,5 +116,6 @@ func (f *VulnerabilityFlagGroup) ToOptions() (VulnerabilityOptions, error) { | |||
| return vex.NewSource(s) | |||
| }), | |||
| SkipVEXRepoUpdate: f.SkipVEXRepoUpdate.Value(), | |||
| SeveritySrc: f.SeveritySrc.Value(), | |||
There was a problem hiding this comment.
I prefer using the explicit type. Is there any advantage of using string?
| SeveritySrc: f.SeveritySrc.Value(), | |
| SeveritySources: xstrings.ToTSlice[dbTypes.SourceID](f.SeveritySources.Value()), |
There was a problem hiding this comment.
I also tried to use dbTypes.SourceID.
But we are using auto value. I think using auto for SourceID is wrong because it is logic and not source.
There was a problem hiding this comment.
I think it's fine. Most of them are source ids. Also, auto can be passed as an argumet of --severity-src.
There was a problem hiding this comment.
okay, i will update logic 👍
There was a problem hiding this comment.
done - 2999049
pkg/vulnerability/vulnerability.go
Outdated
| return dbTypes.SeverityUnknown.String(), "" | ||
| } | ||
|
|
||
| func (c Client) severityInAutoMode(vulnID string, vuln *dbTypes.Vulnerability, dataSource dbTypes.DataSource) (string, dbTypes.SourceID) { |
There was a problem hiding this comment.
What do you think of re-using getSeverity for auto-detection?
diff --git a/pkg/vulnerability/vulnerability.go b/pkg/vulnerability/vulnerability.go
index 5391419b5..710243400 100644
--- a/pkg/vulnerability/vulnerability.go
+++ b/pkg/vulnerability/vulnerability.go
@@ -1,6 +1,7 @@
9E81
package vulnerability
import (
+ "slices"
"strings"
"sync"
@@ -67,7 +68,8 @@ func NewClient(dbc db.Operation) Client {
}
// FillInfo fills extra info in vulnerability objects
-func (c Client) FillInfo(vulns []types.DetectedVulnerability, severitySrc []string) {
+func (c Client) FillInfo(vulns []types.DetectedVulnerability, severitySources []dbTypes.SourceID) {
+ var noSeverityIDs []string
for i := range vulns {
// Add the vulnerability status
// Some vendors such as Red Hat have their own vulnerability status, and we use it.
@@ -90,7 +92,10 @@ func (c Client) FillInfo(vulns []types.DetectedVulnerability, severitySrc []stri
dataSource := lo.FromPtr(vulns[i].DataSource)
// Select the severity according to the detected sourceID.
- severity, severitySource := c.getSeverity(vulnID, &vuln, dataSource, severitySrc)
+ severity, severitySource := c.getSeverity(vulnID, &vuln, dataSource, severitySources)
+ if severity == dbTypes.SeverityUnknown.String() {
+ noSeverityIDs = append(noSeverityIDs, vulnID)
+ }
// The vendor might provide package-specific severity like Debian.
// For example, CVE-2015-2328 in Debian has "unimportant" for mongodb and "low" for pcre3.
@@ -114,42 +119,43 @@ func (c Client) FillInfo(vulns []types.DetectedVulnerability, severitySrc []stri
vulns[i].SeveritySource = severitySource
vulns[i].PrimaryURL = c.getPrimaryURL(vulnID, vuln.References, dataSource.ID)
}
+
+ if !slices.Contains(severitySources, "auto") && len(noSeverityIDs) > 0 {
+ log.Warn("No severity found in specified sources",
+ log.Any("vulnerability-ids", noSeverityIDs), log.Any("severity-sources", severitySources))
+ }
}
-func (c Client) getSeverity(vulnID string, vuln *dbTypes.Vulnerability, dataSource dbTypes.DataSource, severitySrc []string) (string, dbTypes.SourceID) {
- for _, source := range severitySrc {
+func (c Client) getSeverity(vulnID string, vuln *dbTypes.Vulnerability, dataSource dbTypes.DataSource, severitySources []dbTypes.SourceID) (string, dbTypes.SourceID) {
+ for _, source := range severitySources {
if source == "auto" {
- return c.severityInAutoMode(vulnID, vuln, dataSource)
+ return c.autoDetectSeverity(vulnID, vuln, dataSource)
}
- sourceID := dbTypes.SourceID(source)
-
- if severity, ok := vuln.VendorSeverity[sourceID]; ok {
- return severity.String(), sourceID
+ if severity, ok := vuln.VendorSeverity[source]; ok {
+ return severity.String(), source
}
}
- if len(severitySrc) != 1 || severitySrc[0] != "auto" {
- log.Warn("No severity found in specified sources",
- log.String("vulnID", vulnID), log.String("severity sources", strings.Join(severitySrc, ",")))
- }
return dbTypes.SeverityUnknown.String(), ""
}
-func (c Client) severityInAutoMode(vulnID string, vuln *dbTypes.Vulnerability, dataSource dbTypes.DataSource) (string, dbTypes.SourceID) {
- if vs, ok := vuln.VendorSeverity[dataSource.ID]; ok {
- return vs.String(), dataSource.ID
- }
-
- // use severity from GitHub for all GHSA-xxx vulnerabilities
+// autoDetectSeverity detects the severity from the vulnerability ID and data source.
+//
+// The severity is determined in the following order:
+// 1. If the vulnerability is from a specific data source (e.g., Red Hat advisories for Red Hat distributions),
+// use the severity from that data source.
+// 2. For GHSA-IDs, also consider the severity from GitHub Advisory Database.
+// 3. Use the severity from NVD as a fallback.
+// 4. Try severities from other data sources (e.g., Debian severity for Red Hat distributions).
+// 5. If no severity is found from any data source, return "UNKNOWN".
+func (c Client) autoDetectSeverity(vulnID string, vuln *dbTypes.Vulnerability, dataSource dbTypes.DataSource) (string, dbTypes.SourceID) {
+ autoSeveritySrcs := []dbTypes.SourceID{dataSource.ID, vulnerability.NVD}
if strings.HasPrefix(vulnID, "GHSA-") {
- if vs, ok := vuln.VendorSeverity[vulnerability.GHSA]; ok {
- return vs.String(), vulnerability.GHSA
- }
+ // use severity from GitHub for all GHSA-IDs
+ autoSeveritySrcs = append(autoSeveritySrcs, vulnerability.GHSA, vulnerability.NVD)
}
-
- // Try NVD as a fallback if it exists
- if vs, ok := vuln.VendorSeverity[vulnerability.NVD]; ok {
- return vs.String(), vulnerability.NVD
+ if severity, severitySource := c.getSeverity(vulnID, vuln, dataSource, autoSeveritySrcs); severity != dbTypes.SeverityUnknown.String() {
+ return severity, severitySource
}
if vuln.Severity == "" {There was a problem hiding this comment.
looks good.
I used your changes - 413b047
But fixed priority for GHSA
--severity-src flag--vuln-severity-src flag
|
It might be necessary to establish clear guidelines for naming CLI flags. When we want to explicitly indicate that multiple values can be passed, we tend to use the plural form. However, there are cases where this convention isn’t followed consistently. For example, consider the following flags, which accept multiple values, yet one is named in the plural and the other in the singular. This presents a good opportunity to standardize our naming: There are several options:
I examined other tools, and the conventions vary between them. For instance, Docker consistently uses the singular form: In contrast, the AWS CLI always uses the plural form for list inputs: Kubectl shows mixed usage; for example, although the Personally, I don't have a strong preference either way, but I believe it would be beneficial to have a consistent rule within our project. For example, between |
|
only after seeing your message I noticed that we use the full and shortened suffix (source|src). I think plural is more clear. User immediately understands that flag can have multiple values. On the other hand (I could be wrong) - when you use multiple flags - you want the flag names to be as short as possible, so they are easier to paste into the terminal and take up less space (easier to read/check). (But Trivy has config file for case, when user uses many flags) But regardless of form - I like idea of adding standard for flag names. |
|
Opened #8446 |
|
Following #8453 I proposed, |
|
I suppose our team doesn't have a strong opinion on that and will agree on the proposal. Can you please update the flag to |
--vuln-severity-src flag--vuln-severity-source flag
|
Thanks, updated 👍 |
Description
Add
--vuln-severity-sourceflag to set db sources order for vulnerability severity.If severities didn't found in this list - use UNKNOWN severity + show Warning about that.
Examples:
auto(default) mode:severity from another source (nvd):
severity didn't found:
Related issues
--vuln-severity-sourceflag to customize vulnerability severity selection #8180Related PRs
AllSourceIDstrivy-db#485Checklist