Merge pull request #17 from snyk/feat/return-codes

feat: [DGP-503] have extension return 0 for success and 1 for finding…

Author: bsalomon-snyk

internal/commands/ostest/ostest.go

@@ -11,15 +11,19 @@ package ostest
 import (
 	"context"
 	"encoding/json"
+	std_errors "errors"
 	"fmt"
 	"math"
 	"os"
+	"sort"
 
 	"github.com/rs/zerolog"
 	"github.com/snyk/go-application-framework/pkg/apiclients/testapi"
 	"github.com/snyk/go-application-framework/pkg/configuration"
 	"github.com/snyk/go-application-framework/pkg/local_workflows/code_workflow"
 	"github.com/snyk/go-application-framework/pkg/local_workflows/config_utils"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/content_type"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/json_schemas"
 	"github.com/snyk/go-application-framework/pkg/workflow"
 
 	service "github.com/snyk/cli-extension-os-flows/internal/common"
@@ -41,6 +45,12 @@ const FeatureFlagRiskScore = "feature_flag_experimental_risk_score"
 // FeatureFlagRiskScoreInCLI is used to gate the risk score feature in the CLI.
 const FeatureFlagRiskScoreInCLI = "feature_flag_experimental_risk_score_in_cli"
 
+// ApplicationJSONContentType matches the content type for legacy JSON findings records.
+const ApplicationJSONContentType = "application/json"
+
+// ErrNoSummaryData is returned when a test summary cannot be generated due to lack of data.
+var ErrNoSummaryData = std_errors.New("no summary data to create")
+
 // RegisterWorkflows registers the "test" workflow.
 func RegisterWorkflows(e workflow.Engine) error {
 	// Check if workflow already exists
@@ -291,7 +301,105 @@ func runTest(
 		return nil, fmt.Errorf("error converting snyk schema findings to legacy json: %w", err)
 	}
 
-	return []workflow.Data{newWorkflowData("application/json", legacyJSON)}, nil
+	legacyData := NewWorkflowData(ApplicationJSONContentType, legacyJSON)
+	outputData := []workflow.Data{legacyData}
+
+	summaryData, err := NewSummaryData(finalResult, logger, currentDir)
+	if err != nil {
+		if !std_errors.Is(err, ErrNoSummaryData) {
+			logger.Warn().Err(err).Msg("Failed to create test summary for exit code handling")
+		}
+	} else {
+		outputData = append(outputData, summaryData)
+	}
+
+	return outputData, nil
+}
+
+// extractSeverityKeys returns a map of severity keys present in the summaries.
+func extractSeverityKeys(summaries ...*testapi.FindingSummary) map[string]bool {
+	keys := make(map[string]bool)
+	for _, summary := range summaries {
+		if summary != nil && summary.CountBy != nil {
+			if severityCounts, ok := (*summary.CountBy)["severity"]; ok {
+				for severity := range severityCounts {
+					keys[severity] = true
+				}
+			}
+		}
+	}
+	return keys
+}
+
+// getSeverityCount safely retrieves the count for a given severity from a summary.
+func getSeverityCount(summary *testapi.FindingSummary, severity string) uint32 {
+	if summary == nil || summary.CountBy == nil {
+		return 0
+	}
+	if severityCounts, ok := (*summary.CountBy)["severity"]; ok {
+		return severityCounts[severity]
+	}
+	return 0
+}
+
+// NewSummaryData creates a workflow.Data object containing a json_schemas.TestSummary
+// from a testapi.TestResult. This is used for downstream processing, like determining
+// the CLI exit code.
+func NewSummaryData(testResult testapi.TestResult, logger *zerolog.Logger, path string) (workflow.Data, error) {
+	rawSummary := testResult.GetRawSummary()
+	effectiveSummary := testResult.GetEffectiveSummary()
+
+	if rawSummary == nil || effectiveSummary == nil {
+		return nil, fmt.Errorf("test result missing summary information")
+	}
+
+	severityKeys := extractSeverityKeys(rawSummary, effectiveSummary)
+
+	if len(severityKeys) == 0 && rawSummary.Count == 0 {
+		logger.Debug().Msg("No findings in summary, skipping summary creation.")
+		return nil, fmt.Errorf("no findings in summary: %w", ErrNoSummaryData)
+	}
+
+	var summaryResults []json_schemas.TestSummaryResult
+	for severity := range severityKeys {
+		total := getSeverityCount(rawSummary, severity)
+		open := getSeverityCount(effectiveSummary, severity)
+
+		if total > 0 || open > 0 {
+			ignored := 0
+			if total > open {
+				ignored = int(total - open)
+			}
+			summaryResults = append(summaryResults, json_schemas.TestSummaryResult{
+				Severity: severity,
+				Total:    int(total),
+				Open:     int(open),
+				Ignored:  ignored,
+			})
+		}
+	}
+
+	if len(summaryResults) > 0 {
+		// Sort results for consistent output, matching the standard CLI order.
+		sort.Slice(summaryResults, func(i, j int) bool {
+			order := map[string]int{"critical": 4, "high": 3, "medium": 2, "low": 1}
+			return order[summaryResults[i].Severity] > order[summaryResults[j].Severity]
+		})
+
+		testSummary := json_schemas.NewTestSummary("open-source", path)
+		testSummary.Results = summaryResults
+		testSummary.SeverityOrderAsc = []string{"low", "medium", "high", "critical"}
+
+		summaryBytes, err := json.Marshal(testSummary)
+		if err != nil {
+			return nil, fmt.Errorf("failed to marshal test summary: %w", err)
+		}
+
+		summaryWorkflowData := NewWorkflowData(content_type.TEST_SUMMARY, summaryBytes)
+		return summaryWorkflowData, nil
+	}
+
+	return nil, fmt.Errorf("no summary results to process: %w", ErrNoSummaryData)
 }
 
 // runReachabilityFlow handles the reachability analysis flow.
@@ -334,8 +442,8 @@ func createDepGraph(ictx workflow.InvocationContext) (testapi.IoSnykApiV1testdep
 	return depGraphStruct, nil
 }
 
-// Temporary support for dumping findings output to built-in JSON formatter.
-func newWorkflowData(contentType string, data []byte) workflow.Data {
+// NewWorkflowData creates a workflow.Data object with the given content type and data.
+func NewWorkflowData(contentType string, data []byte) workflow.Data {
 	return workflow.NewData(
 		workflow.NewTypeIdentifier(WorkflowID, "ostest"),
 		contentType,

internal/commands/ostest/ostest_summary_test.go

@@ -0,0 +1,236 @@
+package ostest_test
+
+import (
+	"context"
+	"encoding/json"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/google/uuid"
+
+	"github.com/rs/zerolog"
+	"github.com/snyk/go-application-framework/pkg/apiclients/testapi"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/content_type"
+	"github.com/snyk/go-application-framework/pkg/local_workflows/json_schemas"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/snyk/cli-extension-os-flows/internal/commands/ostest"
+)
+
+// mockTestResult is a mock implementation of testapi.TestResult for testing.
+type mockTestResult struct {
+	rawSummary       *testapi.FindingSummary
+	effectiveSummary *testapi.FindingSummary
+}
+
+func (m *mockTestResult) GetRawSummary() *testapi.FindingSummary {
+	return m.rawSummary
+}
+
+func (m *mockTestResult) GetEffectiveSummary() *testapi.FindingSummary {
+	return m.effectiveSummary
+}
+
+// These methods are not used in newSummaryData but are required to satisfy the interface.
+func (m *mockTestResult) GetTestSubject() testapi.TestSubject {
+	return testapi.TestSubject{}
+}
+
+func (m *mockTestResult) Findings(_ context.Context) ([]testapi.FindingData, bool, error) {
+	return nil, false, nil
+}
+
+func (m *mockTestResult) GetExecutionState() testapi.TestExecutionStates {
+	return ""
+}
+
+func (m *mockTestResult) GetTestConfiguration() *testapi.TestConfiguration {
+	return nil
+}
+
+func (m *mockTestResult) GetSubjectLocators() *[]testapi.TestSubjectLocator {
+	return nil
+}
+
+func (m *mockTestResult) GetPassFail() *testapi.PassFail {
+	return nil
+}
+
+func (m *mockTestResult) GetOutcomeReason() *testapi.TestOutcomeReason {
+	return nil
+}
+
+func (m *mockTestResult) GetErrors() *[]testapi.IoSnykApiCommonError {
+	return nil
+}
+
+func (m *mockTestResult) GetWarnings() *[]testapi.IoSnykApiCommonError {
+	return nil
+}
+
+func (m *mockTestResult) GetCreatedAt() *time.Time {
+	return nil
+}
+
+func (m *mockTestResult) GetBreachedPolicies() *testapi.PolicyRefSet {
+	return nil
+}
+
+func (m *mockTestResult) GetTestID() *uuid.UUID {
+	return nil
+}
+
+func (m *mockTestResult) GetID() string {
+	return ""
+}
+
+func Test_newSummaryData(t *testing.T) {
+	logger := zerolog.Nop()
+	path := "/test/path"
+
+	t.Run("no findings should not create summary data, implying exit code 0", func(t *testing.T) {
+		testResult := &mockTestResult{
+			rawSummary:       &testapi.FindingSummary{Count: 0},
+			effectiveSummary: &testapi.FindingSummary{Count: 0},
+		}
+
+		data, err := ostest.NewSummaryData(testResult, &logger, path)
+		assert.Nil(t, data)
+		assert.True(t, errors.Is(err, ostest.ErrNoSummaryData))
+		assert.ErrorContains(t, err, "no findings in summary")
+	})
+
+	t.Run("no open or total findings should not create summary data", func(t *testing.T) {
+		testResult := &mockTestResult{
+			rawSummary: &testapi.FindingSummary{
+				Count: 0,
+				CountBy: &map[string]map[string]uint32{
+					"severity": {"high": 0},
+				},
+			},
+			effectiveSummary: &testapi.FindingSummary{
+				Count: 0,
+				CountBy: &map[string]map[string]uint32{
+					"severity": {"high": 0},
+				},
+			},
+		}
+
+		data, err := ostest.NewSummaryData(testResult, &logger, path)
+		assert.Nil(t, data)
+		assert.True(t, errors.Is(err, ostest.ErrNoSummaryData))
+		assert.ErrorContains(t, err, "no summary results to process")
+	})
+
+	t.Run("one critical finding should create summary data, implying exit code 1", func(t *testing.T) {
+		testResult := &mockTestResult{
+			rawSummary: &testapi.FindingSummary{
+				Count: 1,
+				CountBy: &map[string]map[string]uint32{
+					"severity": {"critical": 1},
+				},
+			},
+			effectiveSummary: &testapi.FindingSummary{
+				Count: 1,
+				CountBy: &map[string]map[string]uint32{
+					"severity": {"critical": 1},
+				},
+			},
+		}
+
+		data, err := ostest.NewSummaryData(testResult, &logger, path)
+		require.NoError(t, err)
+		require.NotNil(t, data)
+
+		assert.Equal(t, content_type.TEST_SUMMARY, data.GetContentType())
+
+		var summary json_schemas.TestSummary
+		payload, ok := data.GetPayload().([]byte)
+		require.True(t, ok)
+		err = json.Unmarshal(payload, &summary)
+		require.NoError(t, err)
+
+		assert.Equal(t, "open-source", summary.Type)
+		assert.Equal(t, path, summary.Path)
+		require.Len(t, summary.Results, 1)
+		assert.Equal(t, "critical", summary.Results[0].Severity)
+		assert.Equal(t, 1, summary.Results[0].Total)
+		assert.Equal(t, 1, summary.Results[0].Open)
+		assert.Equal(t, 0, summary.Results[0].Ignored)
+	})
+
+	t.Run("multiple findings with ignored", func(t *testing.T) {
+		testResult := &mockTestResult{
+			rawSummary: &testapi.FindingSummary{
+				Count: 3,
+				CountBy: &map[string]map[string]uint32{
+					"severity": {
+						"high":   2,
+						"medium": 1,
+					},
+				},
+			},
+			effectiveSummary: &testapi.FindingSummary{
+				Count: 1,
+				CountBy: &map[string]map[string]uint32{
+					"severity": {
+						"high": 1,
+					},
+				},
+			},
+		}
+
+		data, err := ostest.NewSummaryData(testResult, &logger, path)
+		require.NoError(t, err)
+		require.NotNil(t, data)
+
+		// Verify content type is set correctly
+		assert.Equal(t, content_type.TEST_SUMMARY, data.GetContentType())
+
+		var summary json_schemas.TestSummary
+		payload, ok := data.GetPayload().([]byte)
+		require.True(t, ok)
+		err = json.Unmarshal(payload, &summary)
+		require.NoError(t, err)
+
+		require.Len(t, summary.Results, 2)
+		// Results are sorted by severity descending
+		assert.Equal(t, "high", summary.Results[0].Severity)
+		assert.Equal(t, 2, summary.Results[0].Total)
+		assert.Equal(t, 1, summary.Results[0].Open)
+		assert.Equal(t, 1, summary.Results[0].Ignored)
+
+		assert.Equal(t, "medium", summary.Results[1].Severity)
+		assert.Equal(t, 1, summary.Results[1].Total)
+		assert.Equal(t, 0, summary.Results[1].Open)
+		assert.Equal(t, 1, summary.Results[1].Ignored)
+	})
+
+	t.Run("summary is nil", func(t *testing.T) {
+		testResult := &mockTestResult{
+			rawSummary:       nil,
+			effectiveSummary: nil,
+		}
+
+		data, err := ostest.NewSummaryData(testResult, &logger, path)
+		assert.Nil(t, data)
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "test result missing summary information")
+	})
+
+	t.Run("newWorkflowData creates correct content types", func(t *testing.T) {
+		// Test legacy findings content type
+		legacyData := []byte(`{"findings": "data"}`)
+		legacyWorkflowData := ostest.NewWorkflowData(ostest.ApplicationJSONContentType, legacyData)
+		assert.Equal(t, ostest.ApplicationJSONContentType, legacyWorkflowData.GetContentType())
+		assert.Equal(t, legacyData, legacyWorkflowData.GetPayload())
+
+		// Test summary content type
+		summaryData := []byte(`{"summary": "data"}`)
+		summaryWorkflowData := ostest.NewWorkflowData(content_type.TEST_SUMMARY, summaryData)
+		assert.Equal(t, content_type.TEST_SUMMARY, summaryWorkflowData.GetContentType())
+		assert.Equal(t, summaryData, summaryWorkflowData.GetPayload())
+	})
+}

internal/legacy/transform/transform.go

@@ -211,6 +211,7 @@ func ConvertSnykSchemaFindingsToLegacyJSON(params *SnykSchemaToLegacyParams) (js
 		DisplayTargetFile: path,
 		DependencyCount:   int64(params.DepCount),
 		Vulnerabilities:   []definitions.Vulnerability{},
+		Ok:                len(params.Findings) == 0,
 	}
 
 	for _, finding := range params.Findings {