Introduce a shared Finding/SeverityLevel type across scanner integrations

.github/workflows/cgo.yml

@@ -265,13 +265,11 @@ jobs:
       run: |
         set -o pipefail
         # Run tests with JSON output for artifacts, but also show failures
-        go test -v -parallel=8 -timeout=3m -run='${{ matrix.pattern }}' -tags '!integration' -coverprofile=coverage-${{ matrix.shard }}.out -json ./... | tee test-result-unit-${{ matrix.shard }}.json
-
-        # Check if tests failed by looking at JSON output
-        if grep -q '"Action":"fail"' test-result-unit-${{ matrix.shard }}.json; then
-          echo "❌ Tests failed - see output above"
+        if ! go test -v -parallel=8 -timeout=3m -run='${{ matrix.pattern }}' -tags '!integration' -coverprofile=coverage-${{ matrix.shard }}.out -json ./... > test-result-unit-${{ matrix.shard }}.json; then
+          cat test-result-unit-${{ matrix.shard }}.json
           exit 1
         fi
+        cat test-result-unit-${{ matrix.shard }}.json
 
         # Generate coverage HTML report
         go tool cover -html=coverage-${{ matrix.shard }}.out -o coverage-${{ matrix.shard }}.html

docs/adr/54690-shared-scanfindings-type-for-scanner-integrations.md

@@ -0,0 +1,46 @@
+# ADR-54690: Shared Finding/SeverityLevel Type Across Scanner Integrations
+
+**Date**: 2026-08-22
+**Status**: Accepted
+**Deciders**: copilot-swe-agent (PR author), gh-aw maintainers
+
+---
+
+### Context
+
+The codebase integrates nine security scanners (zizmor, poutine, grype, grant, runner-guard, yamllint, audit findings, validation issues, markdown security scanner). Each integration defined its own finding struct with a different severity vocabulary (`High`, `error`, `Negligible`, `note`, …) and its own location shape. As a result, severity classification, message formatting, context-line extraction, and console rendering were reimplemented per tool. This made adding new scanners expensive and meant severity inconsistencies (e.g., `low`-severity zizmor findings rendered as `warning` while grype rendered equivalent findings as `info`) accumulated silently.
+
+### Decision
+
+We will introduce a new `pkg/scanfindings` package that provides a shared `SeverityLevel` enum (`unknown` < `info` < `low` < `medium` < `high` < `critical`) and a shared `Finding` struct (`RuleID`, `Severity`, `Message`, `File`, `Line`, `Column`, `Context`). Each scanner integration retains its own native structs for JSON decoding, then maps onto `scanfindings.Finding` via a small `…FindingsToShared` adapter function. Severity classification, message formatting (`FormatMessage`), context-line extraction (`ContextLines`), rendering (`Render`), sorting (`Sort`), and counting (`CountAtLeast`) are implemented once in `pkg/scanfindings`.
+
+### Alternatives Considered
+
+#### Alternative 1: Extract a shared severity mapping function only
+
+Extract a single `ParseSeverity(raw string) string` helper returning a normalized string, keeping each tool's finding struct and rendering loop separate. This eliminates only the severity inconsistency while leaving context-line extraction, message building, and rendering duplicated across nine files. It is a smaller change but does not solve the maintenance problem that motivated this PR.
+
+#### Alternative 2: Define a `Finding` interface instead of a concrete struct
+
+Define a `Scanner` or `Finding` interface and let each integration implement it via its own native type. This avoids the explicit adapter functions and the coupling to a single shared struct, but adds indirection without eliminating boilerplate: every integration would still implement the same set of methods. The concrete-struct adapter approach chosen here produces less code overall and makes the mapping explicit and testable in isolation.
+
+### Consequences
+
+#### Positive
+- Adding a scanner now requires only a field-mapping adapter; severity classification, rendering, sorting and counting are handled once.
+- The severity vocabulary is normalized through `ParseSeverity`, eliminating per-tool inconsistencies in how identical native labels were classified.
+- `audit_report.go`'s `Finding.Severity` is now a typed `SeverityLevel` rather than an unvalidated `string`, enabling compile-time checks in severity comparisons.
+- Two duplicated rendering loops in `poutine.go` collapse into a single `poutineFindingsToShared` adapter.
+
+#### Negative
+- All scanner integrations are now coupled to `pkg/scanfindings`; changes to the shared type's API propagate to every integration.
+- The `Severity` field type on `audit_report.go`'s `Finding` struct changes from `string` to `SeverityLevel` — callers that compared `finding.Severity == "critical"` must now use `finding.Severity == scanfindings.SeverityCritical` or `.String()`.
+
+#### Neutral
+- Low-severity zizmor and runner-guard findings now render as `info` (matching grype's behavior) rather than `warning` — this is an intentional alignment, not a regression.
+- The `yamllintIssue` internal struct is removed; `parseYamllintLine` returns `scanfindings.Finding` directly.
+- JSON serialization of `SeverityLevel` is unchanged: the underlying type is `string`, so existing JSON consumers reading `"severity"` fields see the same lowercase values.
+
+---
+
+*Accepted on 2026-08-22 after validating the shared type and all scanner adapters.*

pkg/cli/audit_agent_output_test.go

@@ -10,6 +10,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/github/gh-aw/pkg/scanfindings"
 	"github.com/github/gh-aw/pkg/workflow"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -126,7 +127,7 @@ func TestKeyFindingsGeneration(t *testing.T) {
 				}
 				require.NotNil(t, failureFinding,
 					"Expected an error finding with 'Failed' in title for scenario %q", tt.name)
-				assert.Equal(t, "critical", failureFinding.Severity,
+				assert.Equal(t, scanfindings.SeverityCritical, failureFinding.Severity,
 					"Expected critical severity for failure finding in scenario %q", tt.name)
 			}
 

pkg/cli/audit_agentic_analysis.go

@@ -8,6 +8,7 @@ import (
 	"time"
 
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/scanfindings"
 	"github.com/github/gh-aw/pkg/timeutil"
 	"github.com/github/gh-aw/pkg/workflow"
 )
@@ -431,7 +432,7 @@ func generateAgenticAssessmentFindings(assessments []AgenticAssessment) []Findin
 		}
 		findings = append(findings, Finding{
 			Category:    category,
-			Severity:    assessment.Severity,
+			Severity:    scanfindings.ParseSeverity(assessment.Severity),
 			Title:       prettifyAssessmentKind(assessment.Kind),
 			Description: assessment.Summary,
 			Impact:      impact,

pkg/cli/audit_report.go

@@ -16,6 +16,7 @@ import (
 	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/github"
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/scanfindings"
 	"github.com/github/gh-aw/pkg/sliceutil"
 	"github.com/github/gh-aw/pkg/stringutil"
 	"github.com/github/gh-aw/pkg/timeutil"
@@ -64,11 +65,11 @@ type AuditData struct {
 
 // Finding represents a key insight discovered during audit
 type Finding struct {
-	Category    string `json:"category"`         // e.g., "error", "performance", "cost", "tooling"
-	Severity    string `json:"severity"`         // "critical", "high", "medium", "low", "info"
-	Title       string `json:"title"`            // Brief title
-	Description string `json:"description"`      // Detailed description
-	Impact      string `json:"impact,omitempty"` // What impact this has
+	Category    string                     `json:"category"`         // e.g., "error", "performance", "cost", "tooling"
+	Severity    scanfindings.SeverityLevel `json:"severity"`         // shared severity vocabulary
+	Title       string                     `json:"title"`            // Brief title
+	Description string                     `json:"description"`      // Detailed description
+	Impact      string                     `json:"impact,omitempty"` // What impact this has
 }
 
 // Recommendation represents an actionable suggestion

pkg/cli/audit_report_render.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 
 	"github.com/github/gh-aw/pkg/console"
+	"github.com/github/gh-aw/pkg/scanfindings"
 )
 
 // renderJSON outputs the audit data as JSON
@@ -237,7 +238,7 @@ func renderConsoleFindings(findings []Finding) {
 	}
 	fmt.Fprintln(os.Stderr, "  findings:")
 	for _, finding := range findings {
-		fmt.Fprintf(os.Stderr, "    [%s] %s: %s\n", strings.ToUpper(finding.Severity), finding.Title, finding.Description)
+		fmt.Fprintf(os.Stderr, "    [%s] %s: %s\n", strings.ToUpper(finding.Severity.String()), finding.Title, finding.Description)
 	}
 }
 
@@ -440,7 +441,7 @@ func renderConsoleLogsPath(logsPath string) {
 func filterActionableFindings(findings []Finding) []Finding {
 	var result []Finding
 	for _, f := range findings {
-		if f.Severity == "critical" || f.Severity == "high" || f.Severity == "medium" || f.Severity == "low" {
+		if f.Severity.AtLeast(scanfindings.SeverityLow) {
 			result = append(result, f)
 		}
 	}

pkg/cli/audit_report_test.go

@@ -13,6 +13,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/github/gh-aw/pkg/scanfindings"
 	"github.com/github/gh-aw/pkg/testutil"
 	"github.com/github/gh-aw/pkg/workflow"
 	"github.com/stretchr/testify/assert"
@@ -55,7 +56,7 @@ func createTestProcessedRun(opts ...func(*ProcessedRun)) ProcessedRun {
 func assertFindingExists(t *testing.T, findings []Finding, category, severity string, msgAndArgs ...any) {
 	t.Helper()
 	for _, f := range findings {
-		if f.Category == category && f.Severity == severity {
+		if f.Category == category && f.Severity.String() == severity {
 			return // Found it!
 		}
 	}
@@ -142,7 +143,7 @@ func TestGenerateFindings(t *testing.T) {
 			checkFindings: func(t *testing.T, findings []Finding) {
 				finding := findFindingByCategory(findings, "error")
 				require.NotNil(t, finding, "Failed workflow should generate an error finding")
-				assert.Equal(t, "critical", finding.Severity, "Error finding should have critical severity")
+				assert.Equal(t, scanfindings.SeverityCritical, finding.Severity, "Error finding should have critical severity")
 				assert.Contains(t, finding.Title, "Failed", "Error finding should have 'Failed' in title")
 				assert.Contains(t, finding.Description, "Test error", "Error finding description should include the first error message")
 			},
@@ -179,7 +180,7 @@ func TestGenerateFindings(t *testing.T) {
 			checkFindings: func(t *testing.T, findings []Finding) {
 				finding := findFindingByCategory(findings, "error")
 				require.NotNil(t, finding, "Failed workflow should generate an error finding")
-				assert.Equal(t, "critical", finding.Severity, "Error finding should have critical severity")
+				assert.Equal(t, scanfindings.SeverityCritical, finding.Severity, "Error finding should have critical severity")
 				assert.Equal(t, "Workflow 'Test Workflow' failed with 1 error(s)", finding.Description,
 					"Description should match standard format without error message suffix when no errors available")
 			},
@@ -202,7 +203,7 @@ func TestGenerateFindings(t *testing.T) {
 			checkFindings: func(t *testing.T, findings []Finding) {
 				finding := findFindingByCategory(findings, "error")
 				require.NotNil(t, finding, "Failed workflow should generate an error finding")
-				assert.Equal(t, "critical", finding.Severity, "Error finding should have critical severity")
+				assert.Equal(t, scanfindings.SeverityCritical, finding.Severity, "Error finding should have critical severity")
 				assert.Contains(t, finding.Description, "2 error(s)",
 					"Description should reflect the actual number of errors, not metrics.ErrorCount")
 				assert.NotContains(t, finding.Description, "0 error(s)",
@@ -224,7 +225,7 @@ func TestGenerateFindings(t *testing.T) {
 			checkFindings: func(t *testing.T, findings []Finding) {
 				finding := findFindingByCategory(findings, "error")
 				require.NotNil(t, finding, "Failed workflow should still generate an error finding")
-				assert.Equal(t, "critical", finding.Severity, "Error finding should have critical severity")
+				assert.Equal(t, scanfindings.SeverityCritical, finding.Severity, "Error finding should have critical severity")
 				assert.Contains(t, finding.Description, "before agent activation",
 					"Description should indicate pre-activation failure when no logs are available")
 				assert.Contains(t, finding.Description, "no error logs",
@@ -1162,7 +1163,7 @@ func TestFindingSeverityOrdering(t *testing.T) {
 	// Should have critical, high, and medium findings
 	severityCounts := make(map[string]int)
 	for _, f := range findings {
-		severityCounts[f.Severity]++
+		severityCounts[f.Severity.String()]++
 	}
 
 	assert.NotZero(t, severityCounts["critical"],

pkg/cli/grant.go

@@ -15,6 +15,7 @@ import (
 	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/gitutil"
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/scanfindings"
 	"github.com/github/gh-aw/pkg/workflow"
 	"gopkg.in/yaml.v3"
 )
@@ -330,7 +331,17 @@ func grantDisplayFindings(imageTag string, output *grantOutput) (int, error) {
 		return 0, nil
 	}
 
-	total := 0
+	findings := grantFindingsToShared(imageTag, output)
+	scanfindings.Render(os.Stderr, findings)
+
+	return len(findings), nil
+}
+
+// grantFindingsToShared maps grant's denied license packages onto the shared
+// finding representation used by every scanner integration. Container images have
+// no source location, so the image tag is reported as the finding location.
+func grantFindingsToShared(imageTag string, output *grantOutput) []scanfindings.Finding {
+	var findings []scanfindings.Finding
 	for _, target := range output.Run.Targets {
 		for _, pkg := range target.Evaluation.Findings.Packages {
 			if pkg.Decision != "deny" {
@@ -354,22 +365,17 @@ func grantDisplayFindings(imageTag string, output *grantOutput) (int, error) {
 				}
 			}
 
-			message := fmt.Sprintf("license policy violation: %s (%s)", grantPackageRef(pkg), licenses)
-			compilerErr := console.CompilerError{
-				Position: console.ErrorPosition{
-					File:   imageTag,
-					Line:   1,
-					Column: 1,
-				},
-				Type:    "error",
-				Message: message,
-			}
-			fmt.Fprint(os.Stderr, console.FormatError(compilerErr))
-			total++
+			findings = append(findings, scanfindings.Finding{
+				RuleID:   "license-policy",
+				Severity: scanfindings.SeverityHigh,
+				Message:  fmt.Sprintf("license policy violation: %s (%s)", grantPackageRef(pkg), licenses),
+				File:     imageTag,
+				Line:     1,
+				Column:   1,
+			})
 		}
 	}
-
-	return total, nil
+	return findings
 }
 
 func grantPackageRef(pkg grantPackageFinding) string {

pkg/cli/grype.go

@@ -36,6 +36,7 @@ import (
 	"github.com/github/gh-aw/pkg/fileutil"
 	"github.com/github/gh-aw/pkg/gitutil"
 	"github.com/github/gh-aw/pkg/logger"
+	"github.com/github/gh-aw/pkg/scanfindings"
 	"github.com/github/gh-aw/pkg/workflow"
 )
 
@@ -395,45 +396,43 @@ func grypeDisplayFindings(imageTag string, output *grypeOutput) int {
 		return 0
 	}
 
-	for _, match := range output.Matches {
+	findings := grypeFindingsToShared(imageTag, output.Matches)
+	scanfindings.Render(os.Stderr, findings)
+
+	return len(findings)
+}
+
+// grypeFindingsToShared maps grype's native vulnerability matches onto the shared
+// finding representation used by every scanner integration. Container images have
+// no source location, so the image tag is reported as the finding location.
+func grypeFindingsToShared(imageTag string, matches []grypeFinding) []scanfindings.Finding {
+	findings := make([]scanfindings.Finding, 0, len(matches))
+	for _, match := range matches {
 		vuln := match.Vulnerability
 		art := match.Artifact
 
-		severity := vuln.Severity
-		if severity == "" {
-			severity = "Unknown"
-		}
-
-		// Map severity to error type for display purposes.
-		errorType := "warning"
-		switch strings.ToLower(severity) {
-		case "critical", "high":
-			errorType = "error"
-		case "low", "negligible", "informational":
-			errorType = "info"
+		severityLabel := vuln.Severity
+		if severityLabel == "" {
+			severityLabel = "Unknown"
 		}
 
 		// Build a compact message: [Severity] CVE-ID: package@version (fix: x.y.z) (url)
-		message := fmt.Sprintf("[%s] %s: %s@%s", severity, vuln.ID, art.Name, art.Version)
+		message := fmt.Sprintf("[%s] %s: %s@%s", severityLabel, vuln.ID, art.Name, art.Version)
 		if len(vuln.Fix.Versions) > 0 {
 			message = fmt.Sprintf("%s (fix: %s)", message, strings.Join(vuln.Fix.Versions, ", "))
 		}
 		if vuln.DataSource != "" {
 			message = fmt.Sprintf("%s (%s)", message, vuln.DataSource)
 		}
 
-		compilerErr := console.CompilerError{
-			Position: console.ErrorPosition{
-				File:   imageTag,
-				Line:   1,
-				Column: 1,
-			},
-			Type:    errorType,
-			Message: message,
-		}
-
-		fmt.Fprint(os.Stderr, console.FormatError(compilerErr))
+		findings = append(findings, scanfindings.Finding{
+			RuleID:   vuln.ID,
+			Severity: scanfindings.ParseSeverity(severityLabel),
+			Message:  message,
+			File:     imageTag,
+			Line:     1,
+			Column:   1,
+		})
 	}
-
-	return len(output.Matches)
+	return findings
 }