fix(diff/sarif): restore fail-on gating for persisted findings, fix severity badges, decouple icons from severity

internal/cli/cmd_progress.go

@@ -286,8 +286,9 @@ func analyzerProgressDetail(stats sdk.ReachabilityStats) string {
 	return "[" + strings.Join(parts, ", ") + "]"
 }
 
-// diffPolicyOutcomeProgressChild summarizes introduced diff findings using
-// the same introduced-only failure semantics as `bomly diff --audit`.
+// diffPolicyOutcomeProgressChild summarizes the diff findings that gate the
+// run, using the same blocking semantics as `bomly diff --audit` (introduced
+// plus persisted — see auditBlockingFindings).
 func diffPolicyOutcomeProgressChild(audit *diffengine.Audit) progress.Child {
 	if audit == nil {
 		return progress.Child{
@@ -296,21 +297,22 @@ func diffPolicyOutcomeProgressChild(audit *diffengine.Audit) progress.Child {
 			Detail: "[no audit delta]",
 		}
 	}
-	failing := output.FailingFindingCount(audit.Introduced)
-	warnings := len(audit.Introduced) - failing
+	blocking := auditBlockingFindings(audit)
+	failing := output.FailingFindingCount(blocking)
+	warnings := len(blocking) - failing
 	child := progress.Child{
 		Icon:  progress.CheckMark,
 		Label: "Policy Outcome",
 	}
 	switch {
 	case failing > 0:
 		child.Icon = progress.CrossMark
-		child.Detail = fmt.Sprintf("[%d introduced failing, %d warnings]", failing, warnings)
+		child.Detail = fmt.Sprintf("[%d failing, %d warnings]", failing, warnings)
 	case warnings > 0:
 		child.Icon = progress.WarningMark
-		child.Detail = fmt.Sprintf("[passed, %d introduced warnings]", warnings)
+		child.Detail = fmt.Sprintf("[passed, %d warnings]", warnings)
 	default:
-		child.Detail = "[passed, no introduced findings]"
+		child.Detail = "[passed, no findings]"
 	}
 	return child
 }

internal/cli/cmd_progress_test.go

@@ -92,7 +92,23 @@ func TestDiffPolicyOutcomeProgressChild_ReportsIntroducedOutcome(t *testing.T) {
 	if child.Icon != progress.CrossMark {
 		t.Fatalf("expected failing outcome icon, got %#v", child)
 	}
-	if child.Detail != "[1 introduced failing, 1 warnings]" {
+	if child.Detail != "[1 failing, 1 warnings]" {
+		t.Fatalf("unexpected policy outcome detail: %#v", child)
+	}
+}
+
+func TestDiffPolicyOutcomeProgressChild_PersistedFindingsAlsoFail(t *testing.T) {
+	// A persisted finding is tied to a package the diff actually changed, so
+	// it must gate the run exactly like an introduced one.
+	child := diffPolicyOutcomeProgressChild(&diffengine.Audit{
+		Persisted: []sdk.Finding{
+			{Disposition: sdk.FindingDispositionFail},
+		},
+	})
+	if child.Icon != progress.CrossMark {
+		t.Fatalf("expected failing outcome icon for a persisted failing finding, got %#v", child)
+	}
+	if child.Detail != "[1 failing, 0 warnings]" {
 		t.Fatalf("unexpected policy outcome detail: %#v", child)
 	}
 }

internal/cli/diff_cmd.go

@@ -199,7 +199,10 @@ func newDiffCmd() *cobra.Command {
 			}
 			if outputFormat == output.FormatSARIF {
 				prog.Success("Resolved Graph")
-				return sarifRenderer(streams.reportWriter())
+				if err := sarifRenderer(streams.reportWriter()); err != nil {
+					return err
+				}
+				return diffPolicyExit(current.Audit, diffResult.Audit)
 			}
 			if current.Interactive {
 				prog.Stop()
@@ -211,9 +214,9 @@ func newDiffCmd() *cobra.Command {
 				prog.SeparateReport()
 			}
 			err = output.Write(streams.reportWriter(), outputFormat, payload, reportRenderers)
-			if err == nil && current.Audit && diffResult.Audit != nil {
-				if failing := output.FailingFindingCount(diffResult.Audit.Introduced); failing > 0 {
-					return exit.PolicyViolationFindings(failing)
+			if err == nil {
+				if exitErr := diffPolicyExit(current.Audit, diffResult.Audit); exitErr != nil {
+					return exitErr
 				}
 			}
 			return err
@@ -227,16 +230,33 @@ func newDiffCmd() *cobra.Command {
 	return cmd
 }
 
-func diffSARIFFindings(audit *diffengine.Audit) []sdk.Finding {
-	if audit == nil || len(audit.Introduced) == 0 {
+// auditBlockingFindings returns the findings that must keep gating this diff
+// and remain open code-scanning alerts: newly introduced findings, plus
+// persisted ones.
+//
+// Every Persisted finding is, by construction, tied to a package the diff
+// actually changed: the focused audit graph built in engine/diff.Run only
+// ever audits added, removed, or version-changed packages (see
+// focusedAuditGraphs), so a finding can only be classified "persisted" when
+// the same finding key survives a real change to that package. In other
+// words, "persisted" never means unrelated pre-existing debt — it always
+// means "this exact version bump still ships a known issue" — so it must
+// fail the job under --fail-on just like an introduced finding, and must stay
+// in the uploaded SARIF so GitHub doesn't close its alert.
+func auditBlockingFindings(audit *diffengine.Audit) []sdk.Finding {
+	if audit == nil {
 		return nil
 	}
-	return append([]sdk.Finding(nil), audit.Introduced...)
+	return append(append([]sdk.Finding(nil), audit.Introduced...), audit.Persisted...)
+}
+
+func diffSARIFFindings(audit *diffengine.Audit) []sdk.Finding {
+	return auditBlockingFindings(audit)
 }
 
 func diffPolicyExit(auditEnabled bool, audit *diffengine.Audit) error {
 	if auditEnabled && audit != nil {
-		if failing := output.FailingFindingCount(audit.Introduced); failing > 0 {
+		if failing := output.FailingFindingCount(auditBlockingFindings(audit)); failing > 0 {
 			return exit.PolicyViolationFindings(failing)
 		}
 	}

internal/cli/diff_cmd_test.go

@@ -49,7 +49,10 @@ func TestRenderDiffTextShowsFindingsSummaryLine(t *testing.T) {
 	}
 }
 
-func TestDiffSARIFFindingsOnlyIncludesIntroduced(t *testing.T) {
+func TestDiffSARIFFindingsIncludesIntroducedAndPersisted(t *testing.T) {
+	// Persisted findings are always tied to a package the diff changed, so
+	// they must stay in the SARIF output or GitHub closes their alert as if
+	// the issue had been resolved.
 	introduced := sdk.Finding{ID: "new", PackageRef: "pkg:npm/new@1.0.0"}
 	resolved := sdk.Finding{ID: "old", PackageRef: "pkg:npm/old@1.0.0"}
 	persisted := sdk.Finding{ID: "kept", PackageRef: "pkg:npm/kept@1.0.0"}
@@ -59,15 +62,49 @@ func TestDiffSARIFFindingsOnlyIncludesIntroduced(t *testing.T) {
 		Resolved:   []sdk.Finding{resolved},
 		Persisted:  []sdk.Finding{persisted},
 	})
-	if len(got) != 1 || got[0].ID != introduced.ID {
-		t.Fatalf("diffSARIFFindings() = %#v, want only introduced finding", got)
+	gotIDs := make(map[string]bool, len(got))
+	for _, f := range got {
+		gotIDs[f.ID] = true
+	}
+	if len(got) != 2 || !gotIDs[introduced.ID] || !gotIDs[persisted.ID] {
+		t.Fatalf("diffSARIFFindings() = %#v, want introduced + persisted, excluding resolved", got)
 	}
 
 	if got := diffSARIFFindings(&diffengine.Audit{Resolved: []sdk.Finding{resolved}}); len(got) != 0 {
 		t.Fatalf("resolved-only SARIF findings = %#v, want none", got)
 	}
 }
 
+func TestDiffPolicyExit_PersistedFailingFindingsAlsoFailTheJob(t *testing.T) {
+	// Regression: a version bump that doesn't remediate an advisory classifies
+	// as persisted (see PR fixing diff section agreement), and that must still
+	// fail --fail-on any, since the finding is tied to a package this diff
+	// actually changed.
+	err := diffPolicyExit(true, &diffengine.Audit{
+		Persisted: []sdk.Finding{{ID: "CVE-PERSISTS", Disposition: sdk.FindingDispositionFail}},
+	})
+	if err == nil {
+		t.Fatal("expected a policy violation error for a failing persisted finding")
+	}
+}
+
+func TestDiffPolicyExit_PersistedWarningsDoNotFailTheJob(t *testing.T) {
+	err := diffPolicyExit(true, &diffengine.Audit{
+		Persisted: []sdk.Finding{{ID: "license:warn", Disposition: sdk.FindingDispositionWarn}},
+	})
+	if err != nil {
+		t.Fatalf("expected no policy violation for a warning-only persisted finding, got %v", err)
+	}
+}
+
+func TestDiffPolicyExit_NoAuditNoExit(t *testing.T) {
+	if err := diffPolicyExit(false, &diffengine.Audit{
+		Persisted: []sdk.Finding{{ID: "x", Disposition: sdk.FindingDispositionFail}},
+	}); err != nil {
+		t.Fatalf("expected no exit error when audit is disabled, got %v", err)
+	}
+}
+
 func TestRenderDiffTextShowsHighFindingsWhenIntroduced(t *testing.T) {
 	payload := output.DiffResponse{
 		Audit: &output.DiffAudit{
@@ -134,7 +171,7 @@ func TestRenderDiffMarkdownIncludesPatchedVersionsByDefault(t *testing.T) {
 		"| added | react@18.2.0 | 18.2.0 | - | unknown | - |",
 		"| changed | zod | 3.22.0 → 3.23.0 | - | unknown | - |",
 		"## Vulnerabilities",
-		"| ❌ | introduced | HIGH | OSV-123 | react@18.2.0 | 18.2.1 | osv | Prototype pollution in react |",
+		"| introduced | HIGH | OSV-123 | react@18.2.0 | 18.2.1 | osv | Prototype pollution in react |",
 		"## Policy Findings",
 		"| ❌ | introduced | vulnerability | HIGH | OSV-123 | react@18.2.0 | 18.2.1 | Prototype pollution in react |",
 	} {

internal/cli/render/diff_markdown.go

@@ -35,13 +35,18 @@ func diffOverviewMarkdown(payload output.DiffResponse) []string {
 		persisted = len(payload.Audit.Persisted)
 		resolved = len(payload.Audit.Resolved)
 	}
+	// A persisted finding is always tied to a package this diff actually
+	// changed (see auditBlockingFindings in internal/cli/diff_cmd.go), so it
+	// gates the run exactly like an introduced one.
 	status := "✅ Pass"
-	if payload.Audit != nil && outputAuditFailingCount(payload.Audit.Introduced) > 0 {
-		status = "❌ Failing findings introduced"
-	} else if payload.Audit != nil && introduced > 0 {
-		status = "⚠️ Warnings introduced"
-	} else if payload.Audit != nil && outputAuditFailingCount(payload.Audit.Persisted) > 0 {
-		status = "⚠️ Pre-existing findings unresolved"
+	if payload.Audit != nil {
+		failing := outputAuditFailingCount(payload.Audit.Introduced) + outputAuditFailingCount(payload.Audit.Persisted)
+		switch {
+		case failing > 0:
+			status = "❌ Failing findings"
+		case introduced > 0 || persisted > 0:
+			status = "⚠️ Warnings"
+		}
 	}
 	return markdownTable(
 		[]string{"Status", "Manifests", "Dependencies", "Findings", "Duration"},
@@ -169,7 +174,6 @@ func diffVulnerabilityTable(title, status string, changes []output.DiffVulnerabi
 	for _, change := range sortVulnerabilityChanges(changes) {
 		vuln := change.Vulnerability
 		row := []string{
-			findingIcon(status, string(vuln.Severity), ""),
 			status,
 			strings.ToUpper(valueOrDash(string(vuln.Severity))),
 			vuln.ID,
@@ -185,7 +189,7 @@ func diffVulnerabilityTable(title, status string, changes []output.DiffVulnerabi
 		)
 		rows = append(rows, row)
 	}
-	header := []string{"", "Change", "Severity", "ID", "Package"}
+	header := []string{"Change", "Severity", "ID", "Package"}
 	if includeReachability {
 		header = append(header, "Reachability")
 	}
@@ -307,7 +311,7 @@ func diffAuditFindingTable(title, status string, findings []output.AuditFinding,
 	rows := make([][]string, 0, len(findings))
 	for _, finding := range sortDiffAuditFindings(findings) {
 		row := []string{
-			findingIcon(status, string(finding.Severity), string(finding.Disposition)),
+			findingIcon(status, string(finding.Disposition)),
 			status,
 			valueOrDash(finding.Auditor),
 			strings.ToUpper(valueOrDash(string(finding.Severity))),
@@ -338,19 +342,18 @@ func findingIconLegend() []string {
 	return []string{"> **Legend:** ✅ resolved · ❌ failing · ⚠️ warning"}
 }
 
-func findingIcon(status, severity, disposition string) string {
+// findingIcon represents whether a Policy Findings row will fail the run
+// (❌), only warn (⚠️), or has been resolved (✅) — purely a function of
+// status/disposition, never of severity. A Low-severity failing finding still
+// gets ❌; a Critical-severity warning still gets ⚠️.
+func findingIcon(status, disposition string) string {
 	if status == "resolved" {
 		return "✅"
 	}
 	if strings.EqualFold(disposition, "warn") {
 		return "⚠️"
 	}
-	switch strings.ToLower(strings.TrimSpace(severity)) {
-	case "critical", "high":
-		return "❌"
-	default:
-		return "⚠️"
-	}
+	return "❌"
 }
 
 func outputAuditFailingCount(findings []output.AuditFinding) int {

internal/cli/render/diff_markdown_test.go

@@ -9,6 +9,35 @@ import (
 	"github.com/bomly-dev/bomly-cli/sdk"
 )
 
+func TestDiffOverviewMarkdownPersistedFailingCountsAsFailing(t *testing.T) {
+	// A persisted finding is tied to a package the diff actually changed, so
+	// the Overview status must reflect it as a failure, not a softer warning.
+	payload := output.DiffResponse{
+		Audit: &output.DiffAudit{
+			Persisted: []output.AuditFinding{{ID: "CVE-PERSISTS", Disposition: sdk.FindingDispositionFail}},
+		},
+	}
+	got := strings.Join(diffOverviewMarkdown(payload), "\n")
+	if !strings.Contains(got, "❌ Failing findings") {
+		t.Errorf("expected failing status for a failing persisted finding, got %q", got)
+	}
+}
+
+func TestDiffOverviewMarkdownPersistedWarningsAreWarnings(t *testing.T) {
+	payload := output.DiffResponse{
+		Audit: &output.DiffAudit{
+			Persisted: []output.AuditFinding{{ID: "license:warn", Disposition: sdk.FindingDispositionWarn}},
+		},
+	}
+	got := strings.Join(diffOverviewMarkdown(payload), "\n")
+	if !strings.Contains(got, "⚠️ Warnings") {
+		t.Errorf("expected a warning status for a warn-only persisted finding, got %q", got)
+	}
+	if strings.Contains(got, "❌") {
+		t.Errorf("did not expect a failing status, got %q", got)
+	}
+}
+
 func TestHumanizeDurationMS(t *testing.T) {
 	cases := []struct {
 		ms   int64

internal/matchers/osv/mapper.go

@@ -31,7 +31,7 @@ func MapVulnerability(v Vulnerability) sdk.Vulnerability {
 		// Bomly extensions
 		Source:         "osv",
 		Title:          firstNonEmpty(v.Summary, v.ID),
-		ParsedSeverity: extractSeverity(v.Severity),
+		ParsedSeverity: extractSeverity(v.Severity, v.DatabaseSpecific),
 		Reasons:        buildReasons(v),
 		CVSS:           buildCVSS(v.Severity),
 		CWEs:           mapCWEs(v),
@@ -74,10 +74,20 @@ func mapAffected(affected []Affected) []sdk.Affected {
 }
 
 func mapDatabaseSpecific(ds *DatabaseSpecific) map[string]any {
-	if ds == nil || len(ds.CweIDs) == 0 {
+	if ds == nil {
+		return nil
+	}
+	out := map[string]any{}
+	if len(ds.CweIDs) > 0 {
+		out["cwe_ids"] = append([]string(nil), ds.CweIDs...)
+	}
+	if ds.Severity != "" {
+		out["severity"] = ds.Severity
+	}
+	if len(out) == 0 {
 		return nil
 	}
-	return map[string]any{"cwe_ids": append([]string(nil), ds.CweIDs...)}
+	return out
 }
 
 func mapCWEs(v Vulnerability) []sdk.CWE {
@@ -101,7 +111,7 @@ func firstNonEmpty(a, b string) string {
 
 // extractSeverity derives a normalized severity band from OSV severity entries.
 // Prefers CVSS v4 > v3.1 > v3 > v2 > unknown.
-func extractSeverity(severities []Severity) sdk.SeverityLevel {
+func extractSeverity(severities []Severity, ds *DatabaseSpecific) sdk.SeverityLevel {
 	scores := map[string]float64{}
 	for _, s := range severities {
 		if score := parseCVSSScore(s.Type, s.Score); score > 0 {
@@ -113,9 +123,36 @@ func extractSeverity(severities []Severity) sdk.SeverityLevel {
 			return cvssScoreToBand(score)
 		}
 	}
+	// GHSA-sourced advisories (e.g. many Apache project CVEs) frequently
+	// publish no CVSS vector at all, only a textual database_specific.severity
+	// rating. Without this fallback those findings carry SeverityUnknown,
+	// which drops the SARIF security-severity property entirely and leaves
+	// GitHub showing a generic "Warning" badge instead of Low/Medium/High.
+	if ds != nil {
+		if band := severityFromGHSAText(ds.Severity); band != sdk.SeverityUnknown {
+			return band
+		}
+	}
 	return sdk.SeverityUnknown
 }
 
+// severityFromGHSAText maps GitHub Security Advisory's textual severity
+// rating to Bomly's CVSS band vocabulary.
+func severityFromGHSAText(raw string) sdk.SeverityLevel {
+	switch strings.ToUpper(strings.TrimSpace(raw)) {
+	case "CRITICAL":
+		return sdk.SeverityCritical
+	case "HIGH":
+		return sdk.SeverityHigh
+	case "MODERATE", "MEDIUM":
+		return sdk.SeverityMedium
+	case "LOW":
+		return sdk.SeverityLow
+	default:
+		return sdk.SeverityUnknown
+	}
+}
+
 func parseCVSSScore(kind, raw string) float64 {
 	f, err := strconv.ParseFloat(raw, 64)
 	if err == nil {