feat(output): emit PackageLocation.Position in SARIF physicalLocation.region (#60)

The SARIF formatter previously emitted a synthetic location keyed on
the package's qualified name. That gave consumers (GitHub code
scanning, IDE plugins, Azure DevOps) nothing to deep-link to.

This change reads each finding's Package.Locations and emits one
SARIF location per entry:

- artifactLocation.uri = the actual lockfile / manifest path
  (e.g. 'go.mod', 'package-lock.json', 'apps/api/Cargo.lock')
- region.startLine / startColumn / endLine = the line / column /
  end line from PackageLocation.Position, when present

A PackageLocation with a RealPath but no Position still gets a
SARIF location pointing at the file, just without a region — honest
about what we know.

When a finding's package has no Locations at all, the formatter
keeps emitting the prior synthetic location (qualified package name
as URI) so SARIF consumers that key on URI continue to work.

JSON output already exposes Locations through PackageRef (shipped
in C.4); no JSON change needed here.

Tests cover:
- Region populated from Position
- Multiple SARIF locations when a package has multiple
  PackageLocations (monorepo case)
- Fallback to synthetic URI when no Locations
- Location with RealPath but no Position emits no region

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>

Author: bomly-guy

internal/output/sarif.go

@@ -63,13 +63,24 @@ type sarifLocation struct {
 
 type sarifPhysicalLocation struct {
 	ArtifactLocation sarifArtifactLocation `json:"artifactLocation"`
+	Region           *sarifRegion          `json:"region,omitempty"`
 }
 
 type sarifArtifactLocation struct {
 	URI       string `json:"uri"`
 	URIBaseID string `json:"uriBaseId,omitempty"`
 }
 
+// sarifRegion is the SARIF 2.1.0 region descriptor. All numeric
+// fields are 1-based; omitted when unknown. Used to deep-link from
+// a result to the line in the lockfile where the affected
+// dependency is declared.
+type sarifRegion struct {
+	StartLine   int `json:"startLine,omitempty"`
+	StartColumn int `json:"startColumn,omitempty"`
+	EndLine     int `json:"endLine,omitempty"`
+}
+
 type sarifMessage struct {
 	Text string `json:"text"`
 }
@@ -110,21 +121,11 @@ func WriteSARIF(w io.Writer, findings []sdk.Finding, toolName, toolVersion strin
 		if pkgName != "" {
 			msgText = fmt.Sprintf("%s in %s@%s", f.Title, pkgName, pkgVersion)
 		}
-		artifactURI := pkgName
-		if artifactURI == "" {
-			artifactURI = f.ID
-		}
 		results = append(results, sarifResult{
-			RuleID:  f.ID,
-			Level:   severityToSARIFLevel(f.Severity),
-			Message: sarifMessage{Text: msgText},
-			Locations: []sarifLocation{
-				{
-					PhysicalLocation: sarifPhysicalLocation{
-						ArtifactLocation: sarifArtifactLocation{URI: artifactURI},
-					},
-				},
-			},
+			RuleID:    f.ID,
+			Level:     severityToSARIFLevel(f.Severity),
+			Message:   sarifMessage{Text: msgText},
+			Locations: sarifLocationsForFinding(f, pkgName),
 		})
 	}
 
@@ -151,6 +152,62 @@ func WriteSARIF(w io.Writer, findings []sdk.Finding, toolName, toolVersion strin
 	return enc.Encode(log)
 }
 
+// sarifLocationsForFinding builds the SARIF Locations array for a
+// finding. When the finding's package carries one or more
+// PackageLocation entries with a non-nil Position, one SARIF
+// location per entry is emitted with artifactLocation pointing at
+// the source file and a region carrying the line / column. When the
+// package has no positions, a single synthetic location is emitted
+// with the package's qualified name as URI — preserves backward
+// compat for SARIF consumers that already keyed on the package URI.
+//
+// PackageLocations without a Position but with a non-empty RealPath
+// still get a SARIF location with artifactLocation.uri = RealPath
+// and no region. This is honest: we know which file the dep lives
+// in but not exactly where.
+func sarifLocationsForFinding(f sdk.Finding, fallbackURI string) []sarifLocation {
+	if f.Package != nil && len(f.Package.Locations) > 0 {
+		locations := make([]sarifLocation, 0, len(f.Package.Locations))
+		for _, loc := range f.Package.Locations {
+			uri := strings.TrimSpace(loc.RealPath)
+			if uri == "" {
+				uri = strings.TrimSpace(loc.AccessPath)
+			}
+			if uri == "" {
+				continue
+			}
+			pl := sarifPhysicalLocation{
+				ArtifactLocation: sarifArtifactLocation{URI: uri},
+			}
+			if loc.Position != nil && (loc.Position.Line > 0 || loc.Position.Column > 0 || loc.Position.EndLine > 0) {
+				pl.Region = &sarifRegion{
+					StartLine:   loc.Position.Line,
+					StartColumn: loc.Position.Column,
+					EndLine:     loc.Position.EndLine,
+				}
+			}
+			locations = append(locations, sarifLocation{PhysicalLocation: pl})
+		}
+		if len(locations) > 0 {
+			return locations
+		}
+	}
+	// Fallback: emit a synthetic location keyed on the package name
+	// so SARIF consumers always have a non-empty Locations array
+	// (the SARIF spec requires one).
+	uri := strings.TrimSpace(fallbackURI)
+	if uri == "" {
+		uri = f.ID
+	}
+	return []sarifLocation{
+		{
+			PhysicalLocation: sarifPhysicalLocation{
+				ArtifactLocation: sarifArtifactLocation{URI: uri},
+			},
+		},
+	}
+}
+
 func severityToSARIFLevel(severity string) string {
 	switch severity {
 	case "critical", "high":

internal/output/sarif_test.go

@@ -145,3 +145,122 @@ func TestWriteSARIF_OSVHelpURI(t *testing.T) {
 		t.Error("expected OSV help URI in SARIF output")
 	}
 }
+
+func TestWriteSARIF_EmitsRegionFromPackageLocation(t *testing.T) {
+	pkg := &sdk.Package{
+		Name:    "lodash",
+		Version: "4.17.15",
+		Locations: []sdk.PackageLocation{
+			{
+				RealPath:   "package-lock.json",
+				AccessPath: "package-lock.json",
+				Position: &sdk.SourcePosition{
+					File: "package-lock.json",
+					Line: 142,
+				},
+			},
+		},
+	}
+	findings := []sdk.Finding{
+		{ID: "CVE-2021-23337", Kind: sdk.FindingKindVulnerability, Package: pkg, Title: "Vuln", Severity: "high", Source: "osv"},
+	}
+
+	var buf bytes.Buffer
+	if err := WriteSARIF(&buf, findings, "bomly", "0.1.0"); err != nil {
+		t.Fatalf("WriteSARIF: %v", err)
+	}
+	var doc map[string]any
+	if err := json.Unmarshal(buf.Bytes(), &doc); err != nil {
+		t.Fatalf("invalid JSON: %v", err)
+	}
+
+	results := doc["runs"].([]any)[0].(map[string]any)["results"].([]any)
+	if len(results) != 1 {
+		t.Fatalf("results = %d, want 1", len(results))
+	}
+	locations := results[0].(map[string]any)["locations"].([]any)
+	if len(locations) != 1 {
+		t.Fatalf("locations = %d, want 1", len(locations))
+	}
+	pl := locations[0].(map[string]any)["physicalLocation"].(map[string]any)
+	if pl["artifactLocation"].(map[string]any)["uri"] != "package-lock.json" {
+		t.Errorf("artifactLocation.uri = %v, want package-lock.json", pl["artifactLocation"])
+	}
+	region, ok := pl["region"].(map[string]any)
+	if !ok {
+		t.Fatal("expected physicalLocation.region")
+	}
+	if region["startLine"] != float64(142) {
+		t.Errorf("region.startLine = %v, want 142", region["startLine"])
+	}
+}
+
+func TestWriteSARIF_EmitsMultipleLocationsWhenPackageHasSeveral(t *testing.T) {
+	pkg := &sdk.Package{
+		Name:    "express",
+		Version: "4.18.0",
+		Locations: []sdk.PackageLocation{
+			{RealPath: "package-lock.json", Position: &sdk.SourcePosition{File: "package-lock.json", Line: 50}},
+			{RealPath: "apps/api/package-lock.json", Position: &sdk.SourcePosition{File: "apps/api/package-lock.json", Line: 12}},
+		},
+	}
+	findings := []sdk.Finding{
+		{ID: "CVE-test", Kind: sdk.FindingKindVulnerability, Package: pkg, Title: "Vuln", Severity: "high"},
+	}
+	var buf bytes.Buffer
+	if err := WriteSARIF(&buf, findings, "bomly", "0.1.0"); err != nil {
+		t.Fatalf("WriteSARIF: %v", err)
+	}
+	var doc map[string]any
+	_ = json.Unmarshal(buf.Bytes(), &doc)
+	locations := doc["runs"].([]any)[0].(map[string]any)["results"].([]any)[0].(map[string]any)["locations"].([]any)
+	if len(locations) != 2 {
+		t.Fatalf("locations = %d, want 2", len(locations))
+	}
+}
+
+func TestWriteSARIF_FallsBackToPackageNameWhenNoLocations(t *testing.T) {
+	pkg := &sdk.Package{Name: "lodash", Version: "4.17.15"}
+	findings := []sdk.Finding{
+		{ID: "CVE-2021-23337", Kind: sdk.FindingKindVulnerability, Package: pkg, Title: "Vuln", Severity: "high"},
+	}
+	var buf bytes.Buffer
+	if err := WriteSARIF(&buf, findings, "bomly", "0.1.0"); err != nil {
+		t.Fatalf("WriteSARIF: %v", err)
+	}
+	var doc map[string]any
+	_ = json.Unmarshal(buf.Bytes(), &doc)
+	pl := doc["runs"].([]any)[0].(map[string]any)["results"].([]any)[0].(map[string]any)["locations"].([]any)[0].(map[string]any)["physicalLocation"].(map[string]any)
+	if pl["artifactLocation"].(map[string]any)["uri"] != "lodash" {
+		t.Errorf("fallback uri = %v, want package qualified name", pl["artifactLocation"])
+	}
+	if _, hasRegion := pl["region"]; hasRegion {
+		t.Errorf("fallback location should have no region")
+	}
+}
+
+func TestWriteSARIF_LocationWithoutPositionSkipsRegion(t *testing.T) {
+	pkg := &sdk.Package{
+		Name:    "lodash",
+		Version: "4.17.15",
+		Locations: []sdk.PackageLocation{
+			{RealPath: "package-lock.json"},
+		},
+	}
+	findings := []sdk.Finding{
+		{ID: "CVE-2021-23337", Kind: sdk.FindingKindVulnerability, Package: pkg, Title: "Vuln", Severity: "high"},
+	}
+	var buf bytes.Buffer
+	if err := WriteSARIF(&buf, findings, "bomly", "0.1.0"); err != nil {
+		t.Fatalf("WriteSARIF: %v", err)
+	}
+	var doc map[string]any
+	_ = json.Unmarshal(buf.Bytes(), &doc)
+	pl := doc["runs"].([]any)[0].(map[string]any)["results"].([]any)[0].(map[string]any)["locations"].([]any)[0].(map[string]any)["physicalLocation"].(map[string]any)
+	if pl["artifactLocation"].(map[string]any)["uri"] != "package-lock.json" {
+		t.Errorf("uri = %v, want package-lock.json", pl["artifactLocation"])
+	}
+	if _, hasRegion := pl["region"]; hasRegion {
+		t.Error("location without Position should not have a region")
+	}
+}