diff --git a/database/migrations/functions/packages/update_snapshot_security_report.sql b/database/migrations/functions/packages/update_snapshot_security_report.sql index 0b9a2c412d..d9cf53129e 100644 --- a/database/migrations/functions/packages/update_snapshot_security_report.sql +++ b/database/migrations/functions/packages/update_snapshot_security_report.sql @@ -1,25 +1,26 @@ -- update_snapshot_security_report updates the security report of the package's -- snapshot provides. -create or replace function update_snapshot_security_report(p_report jsonb) +create or replace function update_snapshot_security_report( + p_report jsonb, + p_emit_alert boolean +) returns void as $$ declare v_package_id uuid := (p_report->>'package_id')::uuid; v_version text := p_report->>'version'; v_alert_digest text := nullif(p_report->>'alert_digest', ''); - v_previous_alert_digest text; begin - -- Register security alert event for the associated package if the package's - -- version is the latest and the security report's alert digest has changed - select security_report_alert_digest - from snapshot s - join package p using (package_id) - where package_id = v_package_id - and s.version = v_version - and s.version = p.latest_version - into v_previous_alert_digest; - if found then - if v_alert_digest is not null - and (v_previous_alert_digest is null or v_alert_digest <> v_previous_alert_digest) then + -- Register a security alert event when the caller indicates it should be + -- emitted and the scanned version is still the latest + if p_emit_alert and v_alert_digest is not null then + if exists ( + select 1 + from snapshot s + join package p using (package_id) + where package_id = v_package_id + and s.version = v_version + and s.version = p.latest_version + ) then insert into event (package_id, package_version, event_kind_id) values (v_package_id, v_version, 1); end if; diff --git a/database/migrations/schema/064_drop_old_update_snapshot_security_report.sql b/database/migrations/schema/064_drop_old_update_snapshot_security_report.sql new file mode 100644 index 0000000000..f3d8c94487 --- /dev/null +++ b/database/migrations/schema/064_drop_old_update_snapshot_security_report.sql @@ -0,0 +1,5 @@ +drop function if exists update_snapshot_security_report(jsonb); + +---- create above / drop below ---- + +-- Nothing to do diff --git a/database/tests/functions/packages/update_snapshot_security_report.sql b/database/tests/functions/packages/update_snapshot_security_report.sql index 12bfad0971..b23ea8ca83 100644 --- a/database/tests/functions/packages/update_snapshot_security_report.sql +++ b/database/tests/functions/packages/update_snapshot_security_report.sql @@ -81,7 +81,7 @@ select update_snapshot_security_report('{ {"k": "v"} ] } -}'); +}', false); select is(security_report, '{ "quay.io/org/pkg1:1.0.0": [ {"k": "v"} @@ -104,7 +104,7 @@ select update_snapshot_security_report('{ "package_id": "00000000-0000-0000-0000-000000000002", "version": "0.0.9", "alert_digest": "digest-a" -}'); +}', true); select is( count(*)::int, 0::int, @@ -117,11 +117,11 @@ where p.name = 'package2' and e.package_version = '0.0.9'; select update_snapshot_security_report('{ "package_id": "00000000-0000-0000-0000-000000000002", "version": "1.0.0" -}'); +}', false); select is( count(*)::int, 0::int, - 'No security alert event should exist for package 2 version 1.0.0 as the alert digest is null' + 'No security alert event should exist for package 2 version 1.0.0 when emit alert is false' ) from event e join package p using (package_id) @@ -131,7 +131,7 @@ select update_snapshot_security_report('{ "package_id": "00000000-0000-0000-0000-000000000002", "version": "1.0.0", "alert_digest": "digest-b" -}'); +}', true); select is( count(*)::int, 1::int, @@ -145,11 +145,11 @@ select update_snapshot_security_report('{ "package_id": "00000000-0000-0000-0000-000000000002", "version": "1.0.0", "alert_digest": "digest-b" -}'); +}', false); select is( count(*)::int, 1::int, - 'No new security alert event should exist for package 2 version 1.0.0 as the alert digest has not changed' + 'No new security alert event should exist for package 2 version 1.0.0 when emit alert is false' ) from event e join package p using (package_id) @@ -168,7 +168,7 @@ select update_snapshot_security_report('{ "package_id": "00000000-0000-0000-0000-000000000002", "version": "1.1.0", "alert_digest": "digest-b" -}'); +}', true); select is( count(*)::int, 1::int, @@ -182,7 +182,7 @@ select update_snapshot_security_report('{ "package_id": "00000000-0000-0000-0000-000000000002", "version": "1.1.0", "alert_digest": "digest-c" -}'); +}', true); select is( count(*)::int, 2::int, diff --git a/internal/notification/template/security_alert_email.tmpl b/internal/notification/template/security_alert_email.tmpl index ec18f566bc..ef0cb16bf4 100644 --- a/internal/notification/template/security_alert_email.tmpl +++ b/internal/notification/template/security_alert_email.tmpl @@ -46,7 +46,7 @@
Or you can copy-paste this link: {{ .Package.URL }}?modal=security-report&event-id={{ .Event.ID }}
- Please note that security alerts only consider vulnerabilities of high and critical severity. Any time a new potential security vulnerability is detected you'll be notified again. + Please note that security alerts only consider vulnerabilities of high and critical severity. You'll be notified again if a new potential security vulnerability is detected or if an existing one is upgraded from high to critical.
diff --git a/internal/pkg/manager.go b/internal/pkg/manager.go index c0b99d3ad5..da46d597b8 100644 --- a/internal/pkg/manager.go +++ b/internal/pkg/manager.go @@ -3,6 +3,7 @@ package pkg import ( "context" "encoding/json" + "errors" "fmt" "net/url" "sort" @@ -11,7 +12,10 @@ import ( "github.com/Masterminds/semver/v3" "github.com/artifacthub/hub/internal/hub" + "github.com/artifacthub/hub/internal/scanner" "github.com/artifacthub/hub/internal/util" + "github.com/jackc/pgx/v4" + "github.com/rs/zerolog/log" "github.com/satori/uuid" stripmd "github.com/writeas/go-strip-markdown" ) @@ -32,6 +36,7 @@ const ( getPkgsStatsDBQ = `select get_packages_stats()` getProductionUsageDBQ = `select get_production_usage($1::uuid, $2::text, $3::text)` getSnapshotSecurityReportDBQ = `select security_report from snapshot where package_id = $1 and version = $2` + getSnapshotSecurityReportTxDBQ = `select security_report from snapshot where package_id = $1 and version = $2 for update` getSnapshotsToScanDBQ = `select get_snapshots_to_scan()` getRandomPkgsDBQ = `select get_random_packages()` getValuesSchemaDBQ = `select values_schema from snapshot where package_id = $1 and version = $2` @@ -39,7 +44,7 @@ const ( searchPkgsDBQ = `select * from search_packages($1::jsonb)` searchPkgsMonocularDBQ = `select search_packages_monocular($1::text, $2::text)` togglePkgStarDBQ = `select toggle_star($1::uuid, $2::uuid)` - updateSnapshotSecurityReportDBQ = `select update_snapshot_security_report($1::jsonb)` + updateSnapshotSecurityReportDBQ = `select update_snapshot_security_report($1::jsonb, $2::boolean)` unregisterPkgDBQ = `select unregister_package($1::jsonb)` ) @@ -385,10 +390,58 @@ func (m *Manager) UpdateSnapshotSecurityReport(ctx context.Context, r *hub.Snaps return fmt.Errorf("%w: %s", hub.ErrInvalidInput, "version not provided") } - // Update snapshot security report in database - rJSON, _ := json.Marshal(r) - _, err := m.db.Exec(ctx, updateSnapshotSecurityReportDBQ, rJSON) - return err + return util.DBTransact(ctx, m.db, func(tx pgx.Tx) error { + // Lock the snapshot row while computing the alert decision + previousReportJSON, err := getSnapshotSecurityReportJSONForUpdate( + ctx, + tx, + r.PackageID, + r.Version, + ) + if err != nil && !errors.Is(err, hub.ErrNotFound) { + return err + } + if errors.Is(err, hub.ErrNotFound) { + previousReportJSON = nil + } + + // Compare against the stored report to avoid noisy security alerts + emitAlert, err := scanner.ShouldNotifyOnNewOrEscalatedAlerts( + previousReportJSON, + r.ImagesReports, + ) + if err != nil { + log.Error(). + Err(err). + Str("package_id", r.PackageID). + Str("version", r.Version). + Msg("error processing previous security report") + emitAlert = false + } + + // Update snapshot security report in database + rJSON, _ := json.Marshal(r) + _, err = tx.Exec(ctx, updateSnapshotSecurityReportDBQ, rJSON, emitAlert) + return err + }) +} + +// getSnapshotSecurityReportJSONForUpdate returns the stored security report for +// the requested snapshot while holding a row lock for the current transaction. +func getSnapshotSecurityReportJSONForUpdate( + ctx context.Context, + tx pgx.Tx, + pkgID, version string, +) ([]byte, error) { + // Lock the snapshot row so concurrent workers observe the latest report + var dataJSON []byte + if err := tx.QueryRow(ctx, getSnapshotSecurityReportTxDBQ, pkgID, version).Scan(&dataJSON); err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, hub.ErrNotFound + } + return nil, err + } + return dataJSON, nil } // Unregister unregisters the package provided from the database. diff --git a/internal/pkg/manager_test.go b/internal/pkg/manager_test.go index fc7c559ebe..c8ecc75ec4 100644 --- a/internal/pkg/manager_test.go +++ b/internal/pkg/manager_test.go @@ -4,12 +4,14 @@ import ( "context" "encoding/json" "errors" + "fmt" "testing" "time" trivy "github.com/aquasecurity/trivy/pkg/types" "github.com/artifacthub/hub/internal/hub" "github.com/artifacthub/hub/internal/tests" + "github.com/jackc/pgx/v4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -1364,24 +1366,53 @@ func TestToggleStar(t *testing.T) { func TestUpdateSnapshotSecurityReport(t *testing.T) { ctx := context.Background() - r := &hub.SnapshotSecurityReport{ - PackageID: "pkgID", - Version: "1.0.0", - Summary: &hub.SecurityReportSummary{ - High: 2, - Medium: 1, - }, - ImagesReports: map[string]*trivy.Report{ - "organization/image:tag": { - Results: trivy.Results{ - { - Vulnerabilities: nil, - }, - }, + newReport := func(t *testing.T, severity string) *hub.SnapshotSecurityReport { + t.Helper() + + var imageReport *trivy.Report + err := json.Unmarshal([]byte(fmt.Sprintf(` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "%s"} + ] + } + ] + } + `, severity)), &imageReport) + require.NoError(t, err) + + report := &hub.SnapshotSecurityReport{ + PackageID: "pkgID", + Version: "1.0.0", + Summary: &hub.SecurityReportSummary{ + High: 1, }, - }, + ImagesReports: map[string]*trivy.Report{ + "organization/image:tag": imageReport, + }, + } + if severity == "CRITICAL" { + report.Summary.Critical = 1 + report.Summary.High = 0 + } + + return report + } + newStoredReportJSON := func(t *testing.T, severity string) []byte { + t.Helper() + + dataJSON, err := json.Marshal(newReport(t, severity).ImagesReports) + require.NoError(t, err) + + return dataJSON } + + r := newReport(t, "HIGH") + rCritical := newReport(t, "CRITICAL") rJSON, _ := json.Marshal(r) + rCriticalJSON, _ := json.Marshal(rCritical) t.Run("invalid input", func(t *testing.T) { testCases := []struct { @@ -1418,10 +1449,46 @@ func TestUpdateSnapshotSecurityReport(t *testing.T) { } }) + t.Run("database lookup error", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return(nil, tests.ErrFakeDB) + tx.On("Rollback", ctx).Return(nil) + m := NewManager(db) + + err := m.UpdateSnapshotSecurityReport(ctx, r) + assert.Equal(t, tests.ErrFakeDB, err) + db.AssertExpectations(t) + }) + + t.Run("missing previous report is treated as first alert", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return(nil, pgx.ErrNoRows) + tx.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON, true).Return(nil) + tx.On("Commit", ctx).Return(nil) + m := NewManager(db) + + err := m.UpdateSnapshotSecurityReport(ctx, r) + assert.NoError(t, err) + db.AssertExpectations(t) + }) + t.Run("database error", func(t *testing.T) { t.Parallel() db := &tests.DBMock{} - db.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON).Return(tests.ErrFakeDB) + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return(nil, nil) + tx.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON, true).Return(tests.ErrFakeDB) + tx.On("Rollback", ctx).Return(nil) m := NewManager(db) err := m.UpdateSnapshotSecurityReport(ctx, r) @@ -1432,7 +1499,60 @@ func TestUpdateSnapshotSecurityReport(t *testing.T) { t.Run("database update succeeded", func(t *testing.T) { t.Parallel() db := &tests.DBMock{} - db.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON).Return(nil) + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return(nil, nil) + tx.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON, true).Return(nil) + tx.On("Commit", ctx).Return(nil) + m := NewManager(db) + + err := m.UpdateSnapshotSecurityReport(ctx, r) + assert.NoError(t, err) + db.AssertExpectations(t) + }) + + t.Run("database update succeeded without alert", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return(newStoredReportJSON(t, "HIGH"), nil) + tx.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON, false).Return(nil) + tx.On("Commit", ctx).Return(nil) + m := NewManager(db) + + err := m.UpdateSnapshotSecurityReport(ctx, r) + assert.NoError(t, err) + db.AssertExpectations(t) + }) + + t.Run("database update succeeded with severity upgrade", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return(newStoredReportJSON(t, "HIGH"), nil) + tx.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rCriticalJSON, true).Return(nil) + tx.On("Commit", ctx).Return(nil) + m := NewManager(db) + + err := m.UpdateSnapshotSecurityReport(ctx, rCritical) + assert.NoError(t, err) + db.AssertExpectations(t) + }) + + t.Run("invalid previous report suppresses alert", func(t *testing.T) { + t.Parallel() + db := &tests.DBMock{} + tx := &tests.TXMock{} + db.On("Begin", ctx).Return(tx, nil) + tx.On("QueryRow", ctx, getSnapshotSecurityReportTxDBQ, r.PackageID, r.Version). + Return([]byte(`{"invalid"`), nil) + tx.On("Exec", ctx, updateSnapshotSecurityReportDBQ, rJSON, false).Return(nil) + tx.On("Commit", ctx).Return(nil) m := NewManager(db) err := m.UpdateSnapshotSecurityReport(ctx, r) diff --git a/internal/scanner/alerts.go b/internal/scanner/alerts.go new file mode 100644 index 0000000000..fd550529cd --- /dev/null +++ b/internal/scanner/alerts.go @@ -0,0 +1,120 @@ +package scanner + +import ( + "crypto/sha512" + "encoding/json" + "fmt" + "sort" + "strings" + + trivy "github.com/aquasecurity/trivy/pkg/types" +) + +const ( + alertSeverityCritical = "CRITICAL" + alertSeverityHigh = "HIGH" +) + +// BuildAlertDigest generates a digest from the normalized package-level high +// and critical alerts present in the images reports. +func BuildAlertDigest(imagesReports map[string]*trivy.Report) string { + // Collapse repeated findings into a single package-level alert set + alerts := normalizeAlertVulnerabilities(imagesReports) + if len(alerts) == 0 { + return "" + } + + // Sort normalized alerts before hashing to keep the digest stable + normalizedAlerts := make([]string, 0, len(alerts)) + for vulnerabilityID, severity := range alerts { + normalizedAlerts = append( + normalizedAlerts, + fmt.Sprintf("[%s:%s]", severity, vulnerabilityID), + ) + } + sort.Strings(normalizedAlerts) + + return fmt.Sprintf( + "%x", + sha512.Sum512([]byte(strings.Join(normalizedAlerts, ""))), + ) +} + +// ShouldNotifyOnNewOrEscalatedAlerts indicates if the current package-level +// high and critical alerts contain a new vulnerability or a severity upgrade. +func ShouldNotifyOnNewOrEscalatedAlerts( + previousReportJSON []byte, + imagesReports map[string]*trivy.Report, +) (bool, error) { + // Normalize current alerts first so duplicate targets do not affect decisions + currentAlerts := normalizeAlertVulnerabilities(imagesReports) + if len(currentAlerts) == 0 { + return false, nil + } + if isEmptySecurityReport(previousReportJSON) { + return true, nil + } + + // Decode the stored raw report and compare it using the same normalization + var previousReports map[string]*trivy.Report + if err := json.Unmarshal(previousReportJSON, &previousReports); err != nil { + return false, fmt.Errorf( + "error unmarshalling previous security report: %w", + err, + ) + } + previousAlerts := normalizeAlertVulnerabilities(previousReports) + for vulnerabilityID, severity := range currentAlerts { + previousSeverity, ok := previousAlerts[vulnerabilityID] + if !ok || isSeverityUpgrade(previousSeverity, severity) { + return true, nil + } + } + + return false, nil +} + +// isEmptySecurityReport indicates if the stored security report has no data. +func isEmptySecurityReport(reportJSON []byte) bool { + // Normalize empty JSON payload representations produced by the database + report := strings.TrimSpace(string(reportJSON)) + return report == "" || report == "{}" || report == "null" +} + +// isSeverityUpgrade indicates if an alert moved from high to critical. +func isSeverityUpgrade(previousSeverity, currentSeverity string) bool { + return previousSeverity == alertSeverityHigh && + currentSeverity == alertSeverityCritical +} + +// normalizeAlertVulnerabilities builds the package-level alert set used by +// notification comparisons and digest generation. +func normalizeAlertVulnerabilities( + imagesReports map[string]*trivy.Report, +) map[string]string { + alerts := make(map[string]string) + + // Keep one entry per vulnerability and preserve the highest seen severity + for _, imageReport := range imagesReports { + if imageReport == nil { + continue + } + for _, result := range imageReport.Results { + for _, vulnerability := range result.Vulnerabilities { + if vulnerability.VulnerabilityID == "" { + continue + } + switch vulnerability.Severity { + case alertSeverityCritical: + alerts[vulnerability.VulnerabilityID] = vulnerability.Severity + case alertSeverityHigh: + if _, ok := alerts[vulnerability.VulnerabilityID]; !ok { + alerts[vulnerability.VulnerabilityID] = vulnerability.Severity + } + } + } + } + } + + return alerts +} diff --git a/internal/scanner/alerts_test.go b/internal/scanner/alerts_test.go new file mode 100644 index 0000000000..10e51901a0 --- /dev/null +++ b/internal/scanner/alerts_test.go @@ -0,0 +1,329 @@ +package scanner + +import ( + "encoding/json" + "testing" + + trivy "github.com/aquasecurity/trivy/pkg/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildAlertDigest(t *testing.T) { + t.Parallel() + + digestWithDuplicates := BuildAlertDigest(map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"}, + {"VulnerabilityID": "CVE-2", "Severity": "CRITICAL"} + ] + } + ] + } + `), + "image-b": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"}, + {"VulnerabilityID": "CVE-2", "Severity": "CRITICAL"} + ] + } + ] + } + `), + }) + digestNormalized := BuildAlertDigest(map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"}, + {"VulnerabilityID": "CVE-2", "Severity": "CRITICAL"} + ] + } + ] + } + `), + }) + + assert.Equal(t, digestNormalized, digestWithDuplicates) +} + +func TestShouldNotifyOnNewOrEscalatedAlerts(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + currentReports map[string]*trivy.Report + expectedErrMsg string + expectedShouldNotify bool + previousReportJSON []byte + }{ + { + name: "current report has no alerts", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "MEDIUM"} + ] + } + ] + } + `), + }, + expectedShouldNotify: false, + }, + { + name: "first alert detected", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }, + expectedShouldNotify: true, + }, + { + name: "duplicate target churn does not notify", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + "image-b": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }, + expectedShouldNotify: false, + previousReportJSON: mustMarshalImagesReports(t, map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }), + }, + { + name: "new alert notifies", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"}, + {"VulnerabilityID": "CVE-2", "Severity": "HIGH"} + ] + } + ] + } + `), + }, + expectedShouldNotify: true, + previousReportJSON: mustMarshalImagesReports(t, map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }), + }, + { + name: "severity downgrade does not notify", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }, + expectedShouldNotify: false, + previousReportJSON: mustMarshalImagesReports(t, map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "CRITICAL"} + ] + } + ] + } + `), + }), + }, + { + name: "severity upgrade notifies", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "CRITICAL"} + ] + } + ] + } + `), + }, + expectedShouldNotify: true, + previousReportJSON: mustMarshalImagesReports(t, map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }), + }, + { + name: "removed alert does not notify", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }, + expectedShouldNotify: false, + previousReportJSON: mustMarshalImagesReports(t, map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"}, + {"VulnerabilityID": "CVE-2", "Severity": "HIGH"} + ] + } + ] + } + `), + }), + }, + { + name: "invalid previous report", + currentReports: map[string]*trivy.Report{ + "image-a": mustParseReport(t, ` + { + "Results": [ + { + "Vulnerabilities": [ + {"VulnerabilityID": "CVE-1", "Severity": "HIGH"} + ] + } + ] + } + `), + }, + expectedErrMsg: "error unmarshalling previous security report", + previousReportJSON: []byte(`{"invalid"`), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + shouldNotify, err := ShouldNotifyOnNewOrEscalatedAlerts( + tc.previousReportJSON, + tc.currentReports, + ) + + if tc.expectedErrMsg != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), tc.expectedErrMsg) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.expectedShouldNotify, shouldNotify) + }) + } +} + +// mustParseReport unmarshals a Trivy report fixture for tests. +func mustParseReport(t *testing.T, data string) *trivy.Report { + t.Helper() + + var report *trivy.Report + err := json.Unmarshal([]byte(data), &report) + require.NoError(t, err) + + return report +} + +// mustMarshalImagesReports marshals image reports into the stored JSON format. +func mustMarshalImagesReports( + t *testing.T, + imagesReports map[string]*trivy.Report, +) []byte { + t.Helper() + + dataJSON, err := json.Marshal(imagesReports) + require.NoError(t, err) + + return dataJSON +} diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index c6aa6fe576..cf1bf43448 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -3,13 +3,11 @@ package scanner import ( "bytes" "context" - "crypto/sha512" "encoding/json" "errors" "fmt" "os" "os/exec" - "sort" "strings" trivy "github.com/aquasecurity/trivy/pkg/types" @@ -107,7 +105,7 @@ func (s *Scanner) Scan(sn *hub.SnapshotToScan) (*hub.SnapshotSecurityReport, err if len(imagesReports) > 0 { report.ImagesReports = imagesReports report.Summary = generateSummary(imagesReports) - report.AlertDigest = generateAlertDigest(imagesReports) + report.AlertDigest = BuildAlertDigest(imagesReports) } return report, nil @@ -138,28 +136,6 @@ func generateSummary(imagesReports map[string]*trivy.Report) *hub.SecurityReport return summary } -// generateAlertDigest generates an alert digest of the security report from -// the images reports. At the moment the digest is based on the vulnerabilities -// with a severity of high or critical. -func generateAlertDigest(imagesReports map[string]*trivy.Report) string { - var vs []string - for _, imageReport := range imagesReports { - for _, result := range imageReport.Results { - for _, v := range result.Vulnerabilities { - if v.Severity == "HIGH" || v.Severity == "CRITICAL" { - vs = append(vs, fmt.Sprintf("[%s:%s]", v.Severity, v.VulnerabilityID)) - } - } - } - } - var digest string - if len(vs) > 0 { - sort.Strings(vs) - digest = fmt.Sprintf("%x", sha512.Sum512([]byte(strings.Join(vs, "")))) - } - return digest -} - // TrivyScanner is an ImageScanner implementation that uses Trivy to scan // containers images for security vulnerabilities. type TrivyScanner struct {