Fix SARIF validation errors for GitHub CodeQL upload (#384)

Copilot · fproulx-boostsecurity · web-flow · commit a6900f5d6a61 · 2026-01-09T09:46:22.000-05:00
* Initial plan * Fix SARIF validation errors for GitHub CodeQL upload - Set index property on ToolComponentReference to 0 (pointing to first taxonomy) - Set guid property to avoid null serialization - Initialize Rules field in ToolComponent as empty array instead of null - Add comprehensive test case to validate SARIF structure Co-authored-by: fproulx-boostsecurity <76956526+fproulx-boostsecurity@users.noreply.github.com> * Fix lint error and add test for issue #384 - Replace require.Equal with require.InDelta for float comparison to fix testifylint error - Add test fixture with exact YAML from issue #384 - Add comprehensive test case TestSarifFormatIssue384 to validate the fix Co-authored-by: fproulx-boostsecurity <76956526+fproulx-boostsecurity@users.noreply.github.com> * Update test to actually scan and use the issue-384 workflow fixture - Modified TestSarifFormatIssue384 to use InventoryScanner to scan the testdata directory - Test now parses the actual workflow file and runs OPA analysis to generate findings - Added imports for scanner and opa packages - Test validates that issue-384.yml workflow is found and scanned - SARIF output is generated from real scan results instead of mock data Co-authored-by: fproulx-boostsecurity <76956526+fproulx-boostsecurity@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: fproulx-boostsecurity <76956526+fproulx-boostsecurity@users.noreply.github.com>
diff --git a/formatters/sarif/sarif.go b/formatters/sarif/sarif.go
@@ -70,9 +70,13 @@ func (f *Format) Format(ctx context.Context, packages []*models.PackageInsights)
 			Name:         "boost/sast",
 			Version:      &version,
 			Organization: &organization,
+			Rules:        []*sarif.ReportingDescriptor{},
 		}
 
-		taxonomyRef := sarif.NewToolComponentReference().WithName("boost/sast")
+		taxonomyRef := sarif.NewToolComponentReference().
+			WithName("boost/sast").
+			WithIndex(0).
+			WithGuid("00000000-0000-0000-0000-000000000000")
 		run.Tool.Driver.WithSupportedTaxonomies([]*sarif.ToolComponentReference{taxonomyRef})
 
 		run.WithTaxonomies([]*sarif.ToolComponent{taxonomy})
diff --git a/formatters/sarif/sarif_test.go b/formatters/sarif/sarif_test.go
@@ -1,11 +1,108 @@
 package sarif
 
 import (
+	"bytes"
+	"context"
+	"encoding/json"
 	"testing"
 
+	"github.com/boostsecurityio/poutine/models"
+	"github.com/boostsecurityio/poutine/opa"
+	"github.com/boostsecurityio/poutine/results"
+	. "github.com/boostsecurityio/poutine/scanner"
 	"github.com/stretchr/testify/require"
 )
 
+func TestSarifFormat(t *testing.T) {
+	// Create a test package with findings
+	pkg := &models.PackageInsights{
+		Purl:               "pkg:github/test/repo@1.0.0",
+		SourceGitRepo:      "test/repo",
+		SourceGitCommitSha: "abc123",
+		SourceGitRef:       "main",
+		SourceScmType:      "github",
+		FindingsResults: results.FindingsResult{
+			Findings: []results.Finding{
+				{
+					RuleId: "test_rule",
+					Purl:   "pkg:github/test/repo@1.0.0",
+					Meta: results.FindingMeta{
+						Path: ".github/workflows/test.yml",
+						Line: 10,
+					},
+				},
+			},
+			Rules: map[string]results.Rule{
+				"test_rule": {
+					Id:          "test_rule",
+					Title:       "Test Rule",
+					Description: "This is a test rule",
+					Level:       "warning",
+				},
+			},
+		},
+	}
+
+	var buf bytes.Buffer
+	formatter := NewFormat(&buf, "1.0.0")
+	err := formatter.Format(context.Background(), []*models.PackageInsights{pkg})
+	require.NoError(t, err)
+
+	// Parse the generated SARIF
+	var sarifOutput map[string]interface{}
+	err = json.Unmarshal(buf.Bytes(), &sarifOutput)
+	require.NoError(t, err)
+
+	// Validate the structure
+	runs, ok := sarifOutput["runs"].([]interface{})
+	require.True(t, ok, "runs should be an array")
+	require.Len(t, runs, 1)
+
+	run, ok := runs[0].(map[string]interface{})
+	require.True(t, ok)
+
+	// Check tool.driver.supportedTaxonomies
+	tool, ok := run["tool"].(map[string]interface{})
+	require.True(t, ok)
+	driver, ok := tool["driver"].(map[string]interface{})
+	require.True(t, ok)
+
+	supportedTaxonomies, ok := driver["supportedTaxonomies"].([]interface{})
+	require.True(t, ok, "supportedTaxonomies should be an array")
+	require.Len(t, supportedTaxonomies, 1)
+
+	taxonomy, ok := supportedTaxonomies[0].(map[string]interface{})
+	require.True(t, ok)
+
+	// Validate that index is an integer (not null)
+	index, exists := taxonomy["index"]
+	require.True(t, exists, "index field should exist")
+	require.NotNil(t, index, "index should not be null")
+	indexFloat, ok := index.(float64)
+	require.True(t, ok, "index should be a number")
+	require.InDelta(t, float64(0), indexFloat, 0.0001, "index should be 0 for the first taxonomy")
+
+	// Validate that guid is either a string or omitted (not null)
+	if guid, exists := taxonomy["guid"]; exists {
+		require.NotNil(t, guid, "guid should not be null if present")
+	}
+
+	// Check taxonomies array
+	taxonomies, ok := run["taxonomies"].([]interface{})
+	require.True(t, ok, "taxonomies should be an array")
+	require.Len(t, taxonomies, 1)
+
+	taxonomyItem, ok := taxonomies[0].(map[string]interface{})
+	require.True(t, ok)
+
+	// Validate that rules is an array (not null)
+	rulesField, exists := taxonomyItem["rules"]
+	require.True(t, exists, "rules field should exist in taxonomy")
+	require.NotNil(t, rulesField, "rules should not be null")
+	_, ok = rulesField.([]interface{})
+	require.True(t, ok, "rules should be an array")
+}
+
 func TestIsValidGitURL(t *testing.T) {
 	tests := []struct {
 		name    string
@@ -45,3 +142,106 @@ func TestIsValidGitURL(t *testing.T) {
 		})
 	}
 }
+
+// TestSarifFormatIssue384 validates that the fix for issue #384 works correctly.
+// This test scans the actual workflow YAML from the issue report to ensure
+// the SARIF output would be accepted by GitHub's CodeQL upload action.
+func TestSarifFormatIssue384(t *testing.T) {
+	// Scan the testdata directory containing the workflow from issue #384
+	scanner := NewInventoryScanner("testdata")
+	pkg := &models.PackageInsights{
+		Purl:          "pkg:github/coveo/test-repo",
+		SourceGitRepo: "coveo/test-repo",
+		SourceGitRef:  "refs/pull/482/merge",
+		SourceScmType: "github",
+	}
+	err := scanner.Run(pkg)
+	require.NoError(t, err)
+
+	// Verify the workflow was found
+	require.NotEmpty(t, pkg.GithubActionsWorkflows, "should have found the issue-384 workflow")
+
+	// Find the issue-384 workflow
+	var foundWorkflow bool
+	for _, wf := range pkg.GithubActionsWorkflows {
+		if wf.Path == ".github/workflows/issue-384.yml" {
+			foundWorkflow = true
+			break
+		}
+	}
+	require.True(t, foundWorkflow, "should have found issue-384.yml workflow")
+
+	// Analyze with OPA to generate findings (using a basic config)
+	opaInstance, err := opa.NewOpa(context.Background(), &models.Config{
+		Include: []models.ConfigInclude{},
+	})
+	require.NoError(t, err)
+
+	inventory := NewInventory(opaInstance, nil, "", "")
+	scannedPkg, err := inventory.ScanPackage(context.Background(), *pkg, "testdata")
+	require.NoError(t, err)
+
+	// Generate SARIF output
+	var buf bytes.Buffer
+	formatter := NewFormat(&buf, "1.0.0")
+	err = formatter.Format(context.Background(), []*models.PackageInsights{scannedPkg})
+	require.NoError(t, err)
+
+	// Parse the generated SARIF
+	var sarifOutput map[string]interface{}
+	err = json.Unmarshal(buf.Bytes(), &sarifOutput)
+	require.NoError(t, err)
+
+	// Validate critical fields that caused issue #384
+	runs, ok := sarifOutput["runs"].([]interface{})
+	require.True(t, ok, "runs should be an array")
+	require.Len(t, runs, 1)
+
+	run, ok := runs[0].(map[string]interface{})
+	require.True(t, ok)
+
+	// Verify supportedTaxonomies structure
+	tool, ok := run["tool"].(map[string]interface{})
+	require.True(t, ok)
+	driver, ok := tool["driver"].(map[string]interface{})
+	require.True(t, ok)
+
+	supportedTaxonomies, ok := driver["supportedTaxonomies"].([]interface{})
+	require.True(t, ok, "supportedTaxonomies should be an array")
+	require.Len(t, supportedTaxonomies, 1)
+
+	taxonomy, ok := supportedTaxonomies[0].(map[string]interface{})
+	require.True(t, ok)
+
+	// Issue #384: index must be an integer, not null
+	index, exists := taxonomy["index"]
+	require.True(t, exists, "index field should exist")
+	require.NotNil(t, index, "index should not be null (issue #384)")
+	indexFloat, ok := index.(float64)
+	require.True(t, ok, "index should be a number (issue #384)")
+	require.InDelta(t, float64(0), indexFloat, 0.0001, "index should be 0")
+
+	// Issue #384: guid must be a string, not null
+	guid, exists := taxonomy["guid"]
+	require.True(t, exists, "guid field should exist")
+	require.NotNil(t, guid, "guid should not be null (issue #384)")
+	guidStr, ok := guid.(string)
+	require.True(t, ok, "guid should be a string (issue #384)")
+	require.NotEmpty(t, guidStr, "guid should not be empty")
+
+	// Verify taxonomies structure
+	taxonomies, ok := run["taxonomies"].([]interface{})
+	require.True(t, ok, "taxonomies should be an array")
+	require.Len(t, taxonomies, 1)
+
+	taxonomyItem, ok := taxonomies[0].(map[string]interface{})
+	require.True(t, ok)
+
+	// Issue #384: rules must be an array, not null
+	rulesField, exists := taxonomyItem["rules"]
+	require.True(t, exists, "rules field should exist in taxonomy")
+	require.NotNil(t, rulesField, "rules should not be null (issue #384)")
+	rulesArray, ok := rulesField.([]interface{})
+	require.True(t, ok, "rules should be an array (issue #384)")
+	require.NotNil(t, rulesArray, "rules array should not be nil")
+}
diff --git a/formatters/sarif/testdata/.github/workflows/issue-384.yml b/formatters/sarif/testdata/.github/workflows/issue-384.yml
@@ -0,0 +1,33 @@
+name: Issue 384 - SARIF Upload Test
+
+on:
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  poutine:
+    name: Boost Security.io Poutine
+    runs-on:
+      # these are auto-generated
+      - runs-on=${{ github.run_id }}
+      - runner=default_ubuntu_24_arm64
+      - env=${{ vars.RUNS_ON_ENV_DEV }}/region=us-east-1
+
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    steps:
+      - uses: step-security/harden-runner@f4a75cfd619ee5ce8d5b864b0d183aff3c69b55a # v2.13.1
+        with:
+          egress-policy: audit
+      - name: Setup self-hosted runner
+        uses: coveo-platform/setup-runner@v1.0.0
+      - uses: actions/checkout@v5.0.0
+      - name: poutine - GitHub Actions SAST
+        uses: boostsecurityio/poutine-action@61bf0017ee5853beb601609f85c94249b53c26ef
+      - name: Upload poutine SARIF file
+        uses: github/codeql-action/upload-sarif@4fa2a7953630fd2f3fb380f21be14ede0169dd4f # v3.25.12
+        with:
+          sarif_file: results.sarif
