diff --git a/formatters/sarif/sarif.go b/formatters/sarif/sarif.go index f67e9df..c4d8404 100644 --- a/formatters/sarif/sarif.go +++ b/formatters/sarif/sarif.go @@ -99,10 +99,17 @@ func (f *Format) Format(ctx context.Context, packages []*models.PackageInsights) } pkgFindings := findingsByPurl[pkg.Purl] - for _, depPurl := range pkg.PackageDependencies { - normalizedDepPurl := normalizePurl(depPurl) - if depFindings, exists := findingsByPurl[normalizedDepPurl]; exists { - pkgFindings = append(pkgFindings, depFindings...) + seenPurls := map[string]bool{normalizePurl(pkg.Purl): true} + for _, depSlice := range [][]string{pkg.PackageDependencies, pkg.BuildDependencies} { + for _, depPurl := range depSlice { + normalizedDepPurl := normalizePurl(depPurl) + if seenPurls[normalizedDepPurl] { + continue + } + seenPurls[normalizedDepPurl] = true + if depFindings, exists := findingsByPurl[normalizedDepPurl]; exists { + pkgFindings = append(pkgFindings, depFindings...) + } } } diff --git a/formatters/sarif/sarif_test.go b/formatters/sarif/sarif_test.go index 84dcb4c..6203ab7 100644 --- a/formatters/sarif/sarif_test.go +++ b/formatters/sarif/sarif_test.go @@ -13,6 +13,92 @@ import ( "github.com/stretchr/testify/require" ) +// TestSarifFormatBuildDependencyFindings validates that findings keyed by a +// build dependency purl are included in the SARIF output. This exercises +// the formatter's BuildDependencies lookup path. +func TestSarifFormatBuildDependencyFindings(t *testing.T) { + actionPurl := "pkg:githubactions/unverified-owner/some-action" + pkgPurl := "pkg:github/test/repo@main" + pkg := &models.PackageInsights{ + Purl: pkgPurl, + SourceGitRepo: "test/repo", + SourceGitRef: "main", + SourceScmType: "github", + BuildDependencies: []string{actionPurl + "@v1.0"}, + FindingsResults: results.FindingsResult{ + Findings: []results.Finding{ + { + RuleId: "github_action_from_unverified_creator_used", + // Finding keyed by the build dependency purl (version stripped + // by the rego rule). This tests the BuildDependencies lookup. + Purl: actionPurl, + Meta: results.FindingMeta{ + Path: ".github/workflows/ci.yml", + Line: 5, + Job: "build", + Step: "2", + Details: "unverified-owner/some-action@v1.0", + }, + }, + }, + Rules: map[string]results.Rule{ + "github_action_from_unverified_creator_used": { + Id: "github_action_from_unverified_creator_used", + Title: "Github Action from Unverified Creator used", + Description: "Usage of GitHub Actions from unverified creators was detected.", + Level: "note", + }, + }, + }, + } + + var buf bytes.Buffer + formatter := NewFormat(&buf, "1.0.0") + err := formatter.Format(context.Background(), []*models.PackageInsights{pkg}) + require.NoError(t, err) + + var sarifOutput map[string]interface{} + err = json.Unmarshal(buf.Bytes(), &sarifOutput) + require.NoError(t, err) + + runs, ok := sarifOutput["runs"].([]interface{}) + require.True(t, ok) + require.Len(t, runs, 1) + + run, ok := runs[0].(map[string]interface{}) + require.True(t, ok) + + sarifResults, ok := run["results"].([]interface{}) + require.True(t, ok, "results should be present in the SARIF run") + require.Len(t, sarifResults, 1, "build dependency finding should appear in SARIF output") + + result, ok := sarifResults[0].(map[string]interface{}) + require.True(t, ok) + require.Equal(t, "github_action_from_unverified_creator_used", result["ruleId"]) + + locations, ok := result["locations"].([]interface{}) + require.True(t, ok, "locations should be present") + require.Len(t, locations, 1) + + location, ok := locations[0].(map[string]interface{}) + require.True(t, ok) + + physicalLocation, ok := location["physicalLocation"].(map[string]interface{}) + require.True(t, ok) + + artifactLocation, ok := physicalLocation["artifactLocation"].(map[string]interface{}) + require.True(t, ok) + uri, ok := artifactLocation["uri"].(string) + require.True(t, ok, "uri should be a string") + require.Equal(t, ".github/workflows/ci.yml", uri, "uri should be the workflow file path") + + region, ok := physicalLocation["region"].(map[string]interface{}) + require.True(t, ok) + startLine, ok := region["startLine"].(float64) + require.True(t, ok, "startLine should be a number") + require.InDelta(t, float64(5), startLine, 0.0001, "startLine should match the finding line") +} + func TestSarifFormat(t *testing.T) { // Create a test package with findings pkg := &models.PackageInsights{ diff --git a/opa/rego/rules/github_action_from_unverified_creator_used.rego b/opa/rego/rules/github_action_from_unverified_creator_used.rego index a59f2e4..0e9641e 100644 --- a/opa/rego/rules/github_action_from_unverified_creator_used.rego +++ b/opa/rego/rules/github_action_from_unverified_creator_used.rego @@ -17,17 +17,34 @@ github_verified_partners contains p if some p in ["1password", "42crunch", "acti # Consider input package namespaces as verified github_verified_partners contains input.packages[_].package_namespace -results contains poutine.finding( - rule, - repo_purl, - {"details": sprintf("Used in %d repo(s)", [count(unverified_github_actions[repo_purl])])}, -) - -unverified_github_actions[action_repo] contains pkg.purl if { +results contains poutine.finding(rule, pkg.purl, { + "path": workflow.path, + "line": step.lines.uses, + "job": job.id, + "step": i, + "details": step.uses, + "event_triggers": [event | event := workflow.events[j].name], +}) if { pkg := input.packages[_] - dep := array.concat(pkg.build_dependencies, pkg.package_dependencies)[_] + workflow := pkg.github_actions_workflows[_] + job := workflow.jobs[_] + step := job.steps[i] + dep := purl.parse_github_actions(step.uses, pkg.source_git_repo, pkg.source_git_ref) startswith(dep, "pkg:githubactions/") + not regex.match(sprintf("pkg:githubactions/(%s)/", [concat("|", github_verified_partners)]), dep) +} - action_repo := split(dep, "@")[0] +results contains poutine.finding(rule, pkg.purl, { + "path": action.path, + "line": step.lines.uses, + "step": i, + "details": step.uses, +}) if { + pkg := input.packages[_] + action := pkg.github_actions_metadata[_] + action.runs.using == "composite" + step := action.runs.steps[i] + dep := purl.parse_github_actions(step.uses, pkg.source_git_repo, pkg.source_git_ref) + startswith(dep, "pkg:githubactions/") not regex.match(sprintf("pkg:githubactions/(%s)/", [concat("|", github_verified_partners)]), dep) } diff --git a/opa/rego/rules/known_vulnerability_in_build_platform.rego b/opa/rego/rules/known_vulnerability_in_build_platform.rego index 5edbe33..69ce6e9 100644 --- a/opa/rego/rules/known_vulnerability_in_build_platform.rego +++ b/opa/rego/rules/known_vulnerability_in_build_platform.rego @@ -15,10 +15,11 @@ import rego.v1 rule := poutine.rule(rego.metadata.chain()) -results contains poutine.finding(rule, input.provider, { +results contains poutine.finding(rule, pkg.purl, { "osv_id": advisory.osv_id, "details": sprintf("Provider: %s", [input.provider]), }) if { + pkg := input.packages[_] advisory := advisories[input.provider][osv_id] regex.match("^[0-9]+(\\.[0-9]+)*?$", input.version) semver.constraint_check(advisory.vulnerable_version_ranges[_], input.version) diff --git a/scanner/inventory_test.go b/scanner/inventory_test.go index 1a3988d..ab2f8d9 100644 --- a/scanner/inventory_test.go +++ b/scanner/inventory_test.go @@ -335,9 +335,38 @@ func TestFindings(t *testing.T) { }, { RuleId: "github_action_from_unverified_creator_used", - Purl: "pkg:githubactions/kartverket/github-workflows", + Purl: purl, Meta: results.FindingMeta{ - Details: "Used in 1 repo(s)", + Path: ".github/workflows/valid.yml", + Line: 35, + Job: "build", + Step: "4", + Details: "kartverket/github-workflows/.github/workflows/run-terraform.yml@main", + EventTriggers: []string{"push", "pull_request_target"}, + }, + }, + { + RuleId: "github_action_from_unverified_creator_used", + Purl: purl, + Meta: results.FindingMeta{ + Path: ".github/workflows/valid.yml", + Line: 39, + Job: "build", + Step: "5", + Details: "kartverket/github-workflows/.github/workflows/run-terraform.yml@v2.7.1", + EventTriggers: []string{"push", "pull_request_target"}, + }, + }, + { + RuleId: "github_action_from_unverified_creator_used", + Purl: purl, + Meta: results.FindingMeta{ + Path: ".github/workflows/valid.yml", + Line: 43, + Job: "build", + Step: "6", + Details: "kartverket/github-workflows/.github/workflows/run-terraform.yml@v2.2", + EventTriggers: []string{"push", "pull_request_target"}, }, }, { @@ -617,9 +646,14 @@ func TestFindings(t *testing.T) { }, { RuleId: "github_action_from_unverified_creator_used", - Purl: "pkg:githubactions/some/action", + Purl: purl, Meta: results.FindingMeta{ - Details: "Used in 1 repo(s)", + Path: ".github/workflows/test_new_fields.yml", + Line: 44, + Job: "vulnerable_checkout", + Step: "3", + Details: "some/action@v1", + EventTriggers: []string{"pull_request_target"}, }, }, { @@ -747,6 +781,42 @@ func TestRulesConfig(t *testing.T) { assert.Empty(t, labels) } +// TestKnownVulnerabilityInBuildPlatformFinding validates that findings from the +// known_vulnerability_in_build_platform rule use pkg.purl as the finding purl (not +// input.provider), so they are not silently dropped from SARIF output. +func TestKnownVulnerabilityInBuildPlatformFinding(t *testing.T) { + o, _ := opa.NewOpa(context.TODO(), &models.Config{ + Include: []models.ConfigInclude{}, + }) + // provider="gitlab", version="17.3.0" matches advisory PVE-2024-00001 (>=17.3, <17.3.3) + i := NewInventory(o, nil, "gitlab", "17.3.0") + purl := "pkg:github/org/owner" + pkg := &models.PackageInsights{ + Purl: purl, + SourceGitRepo: "org/owner", + SourceGitRef: "main", + } + _ = pkg.NormalizePurl() + + scannedPackage, err := i.ScanPackage(context.Background(), *pkg, "testdata") + require.NoError(t, err) + + var platformFinding *results.Finding + for idx, f := range scannedPackage.FindingsResults.Findings { + if f.RuleId == "known_vulnerability_in_build_platform" { + platformFinding = &scannedPackage.FindingsResults.Findings[idx] + break + } + } + + require.NotNil(t, platformFinding, "expected a known_vulnerability_in_build_platform finding") + // The finding purl must be pkg.purl (not input.provider like "gitlab"), so it + // is not dropped from SARIF output by the formatter's purl-based lookup. + assert.Equal(t, purl, platformFinding.Purl, "finding purl should be pkg.purl, not the provider string") + assert.NotEmpty(t, platformFinding.Meta.OsvId, "osv_id should be populated") + assert.NotEmpty(t, platformFinding.Meta.Details, "details should be populated") +} + func TestStructuredFindingFields(t *testing.T) { o, _ := opa.NewOpa(context.TODO(), &models.Config{ Include: []models.ConfigInclude{},