From 6dc38ed698fb53f867dab6ec15c16b26484fc6eb Mon Sep 17 00:00:00 2001 From: Tim Pickles Date: Wed, 7 Jan 2026 12:34:23 +0000 Subject: [PATCH] feat: show warnings for failed projects for --all-projects --- pkg/depgraph/flags.go | 62 +++++----- pkg/depgraph/legacy_resolution.go | 19 ++- pkg/depgraph/legacy_resolution_test.go | 32 +++++ pkg/depgraph/parsers/depgraph_output.go | 1 + pkg/depgraph/parsers/jsonl_output_parser.go | 2 + pkg/depgraph/sbom_resolution.go | 68 +++++++++-- pkg/depgraph/sbom_resolution_test.go | 111 +++++++++++++++++- .../jsonl_dep_graph_with_error_output | 2 + 8 files changed, 255 insertions(+), 42 deletions(-) create mode 100644 pkg/depgraph/testdata/jsonl_dep_graph_with_error_output diff --git a/pkg/depgraph/flags.go b/pkg/depgraph/flags.go index 06860a6..f5d9177 100644 --- a/pkg/depgraph/flags.go +++ b/pkg/depgraph/flags.go @@ -7,36 +7,37 @@ const ( ) const ( - FlagFailFast = "fail-fast" - FlagAllProjects = "all-projects" - FlagDev = "dev" - FlagExclude = "exclude" - FlagFile = "file" - FlagDetectionDepth = "detection-depth" - FlagPruneRepeatedSubdependencies = "prune-repeated-subdependencies" - FlagMavenAggregateProject = "maven-aggregate-project" - FlagScanUnmanaged = "scan-unmanaged" - FlagScanAllUnmanaged = "scan-all-unmanaged" - FlagSubProject = "sub-project" - FlagGradleSubProject = "gradle-sub-project" - FlagGradleNormalizeDeps = "gradle-normalize-deps" - FlagAllSubProjects = "all-sub-projects" - FlagConfigurationMatching = "configuration-matching" - FlagConfigurationAttributes = "configuration-attributes" - FlagInitScript = "init-script" - FlagYarnWorkspaces = "yarn-workspaces" - FlagPythonCommand = "command" - FlagPythonSkipUnresolved = "skip-unresolved" - FlagPythonPackageManager = "package-manager" - FlagNPMStrictOutOfSync = "strict-out-of-sync" - FlagNugetAssetsProjectName = "assets-project-name" - FlagNugetPkgsFolder = "packages-folder" - FlagUnmanagedMaxDepth = "max-depth" - FlagIncludeProvenance = "include-provenance" - FlagUseSBOMResolution = "use-sbom-resolution" - FlagPrintEffectiveGraph = "effective-graph" - FlagDotnetRuntimeResolution = "dotnet-runtime-resolution" - FlagDotnetTargetFramework = "dotnet-target-framework" + FlagFailFast = "fail-fast" + FlagAllProjects = "all-projects" + FlagDev = "dev" + FlagExclude = "exclude" + FlagFile = "file" + FlagDetectionDepth = "detection-depth" + FlagPruneRepeatedSubdependencies = "prune-repeated-subdependencies" + FlagMavenAggregateProject = "maven-aggregate-project" + FlagScanUnmanaged = "scan-unmanaged" + FlagScanAllUnmanaged = "scan-all-unmanaged" + FlagSubProject = "sub-project" + FlagGradleSubProject = "gradle-sub-project" + FlagGradleNormalizeDeps = "gradle-normalize-deps" + FlagAllSubProjects = "all-sub-projects" + FlagConfigurationMatching = "configuration-matching" + FlagConfigurationAttributes = "configuration-attributes" + FlagInitScript = "init-script" + FlagYarnWorkspaces = "yarn-workspaces" + FlagPythonCommand = "command" + FlagPythonSkipUnresolved = "skip-unresolved" + FlagPythonPackageManager = "package-manager" + FlagNPMStrictOutOfSync = "strict-out-of-sync" + FlagNugetAssetsProjectName = "assets-project-name" + FlagNugetPkgsFolder = "packages-folder" + FlagUnmanagedMaxDepth = "max-depth" + FlagIncludeProvenance = "include-provenance" + FlagUseSBOMResolution = "use-sbom-resolution" + FlagPrintEffectiveGraph = "effective-graph" + FlagPrintEffectiveGraphWithErrors = "effective-graph-with-errors" + FlagDotnetRuntimeResolution = "dotnet-runtime-resolution" + FlagDotnetTargetFramework = "dotnet-target-framework" ) func getFlagSet() *pflag.FlagSet { @@ -70,6 +71,7 @@ func getFlagSet() *pflag.FlagSet { flagSet.Bool(FlagIncludeProvenance, false, "Include checksums in purl to support package provenance.") flagSet.Bool(FlagUseSBOMResolution, false, "Use SBOM resolution instead of legacy CLI.") flagSet.Bool(FlagPrintEffectiveGraph, false, "Return the pruned dependency graph.") + flagSet.Bool(FlagPrintEffectiveGraphWithErrors, false, "Return errors in the pruned dependency graph output.") flagSet.Bool(FlagDotnetRuntimeResolution, false, "Required. You must use this option when you test .NET projects using Runtime Resolution Scanning.") flagSet.String(FlagDotnetTargetFramework, "", "Optional. You may use this option if your solution contains multiple directives. "+ diff --git a/pkg/depgraph/legacy_resolution.go b/pkg/depgraph/legacy_resolution.go index 52bfc73..a447e7b 100644 --- a/pkg/depgraph/legacy_resolution.go +++ b/pkg/depgraph/legacy_resolution.go @@ -5,6 +5,7 @@ import ( "strconv" "github.com/rs/zerolog" + "github.com/snyk/error-catalog-golang-public/snyk_errors" "github.com/snyk/go-application-framework/pkg/configuration" "github.com/snyk/go-application-framework/pkg/workflow" @@ -38,7 +39,7 @@ func handleLegacyResolution(ctx workflow.InvocationContext, config configuration return nil, errNoDepGraphsFound } - workflowOutputData := mapToWorkflowData(depGraphs) + workflowOutputData := mapToWorkflowData(depGraphs, logger) logger.Printf("DepGraph workflow done (extracted %d dependency graphs)", len(workflowOutputData)) return workflowOutputData, nil } @@ -48,10 +49,14 @@ func chooseGraphArgument(config configuration.Configuration) (string, parsers.Ou return "--print-effective-graph", parsers.NewJSONL() } + if config.GetBool(FlagPrintEffectiveGraphWithErrors) { + return "--print-effective-graph-with-errors", parsers.NewJSONL() + } + return "--print-graph", parsers.NewPlainText() } -func mapToWorkflowData(depGraphs []parsers.DepGraphOutput) []workflow.Data { +func mapToWorkflowData(depGraphs []parsers.DepGraphOutput, logger *zerolog.Logger) []workflow.Data { depGraphList := []workflow.Data{} for _, depGraph := range depGraphs { data := workflow.NewData(DataTypeID, contentTypeJSON, depGraph.DepGraph) @@ -63,6 +68,16 @@ func mapToWorkflowData(depGraphs []parsers.DepGraphOutput) []workflow.Data { if depGraph.Target != nil { data.SetMetaData(MetaKeyTarget, string(depGraph.Target)) } + if depGraph.Error != nil { + snykErrors, err := snyk_errors.FromJSONAPIErrorBytes(depGraph.Error) + if err != nil { + logger.Printf("failed to parse error from depgraph output: %v", err) + } else { + for i := range len(snykErrors) { + data.AddError(snykErrors[i]) + } + } + } depGraphList = append(depGraphList, data) } return depGraphList diff --git a/pkg/depgraph/legacy_resolution_test.go b/pkg/depgraph/legacy_resolution_test.go index 430abe9..453008a 100644 --- a/pkg/depgraph/legacy_resolution_test.go +++ b/pkg/depgraph/legacy_resolution_test.go @@ -20,6 +20,9 @@ var payload string //go:embed testdata/jsonl_output var jsonlPayload string +//go:embed testdata/jsonl_dep_graph_with_error_output +var jsonlDepGraphWithErrorPayload string + //go:embed testdata/expected_dep_graph.json var expectedDepGraph string @@ -268,6 +271,35 @@ func Test_LegacyResolution(t *testing.T) { // assert assert.ErrorIs(t, err, errNoDepGraphsFound) }) + + t.Run("should include errors from dep graphs in workflow data", func(t *testing.T) { + config.Set(FlagPrintEffectiveGraphWithErrors, true) + + dataIdentifier := workflow.NewTypeIdentifier(WorkflowID, workflowIDStr) + data := workflow.NewData( + dataIdentifier, + contentTypeJSON, + []byte(jsonlDepGraphWithErrorPayload)) + engineMock. + EXPECT(). + InvokeWithConfig(legacyWorkflowID, config). + Return([]workflow.Data{data}, nil). + Times(1) + + depGraphs, err := handleLegacyResolution(invocationContextMock, config, &nopLogger) + require.Nil(t, err) + require.Len(t, depGraphs, 2) + + verifyMeta(t, depGraphs[0], MetaKeyNormalisedTargetFile, "some normalised target file") + + // verify error + verifyMeta(t, depGraphs[1], MetaKeyNormalisedTargetFile, "some normalised target file") + errorList := depGraphs[1].GetErrorList() + require.Len(t, errorList, 1) + assert.Equal(t, "SNYK-CLI-0000", errorList[0].ErrorCode) + assert.Equal(t, "Unspecified Error", errorList[0].Title) + assert.Equal(t, "Something went wrong", errorList[0].Detail) + }) } func invokeWithConfigAndGetTestCmdArgs( diff --git a/pkg/depgraph/parsers/depgraph_output.go b/pkg/depgraph/parsers/depgraph_output.go index b0d9ff5..67a6ca2 100644 --- a/pkg/depgraph/parsers/depgraph_output.go +++ b/pkg/depgraph/parsers/depgraph_output.go @@ -6,4 +6,5 @@ type DepGraphOutput struct { TargetFileFromPlugin *string Target []byte DepGraph []byte + Error []byte } diff --git a/pkg/depgraph/parsers/jsonl_output_parser.go b/pkg/depgraph/parsers/jsonl_output_parser.go index 7cf9ac5..e71b6d6 100644 --- a/pkg/depgraph/parsers/jsonl_output_parser.go +++ b/pkg/depgraph/parsers/jsonl_output_parser.go @@ -22,6 +22,7 @@ type jsonLine struct { NormalisedTargetFile string `json:"normalisedTargetFile"` TargetFileFromPlugin *string `json:"targetFileFromPlugin"` Target json.RawMessage `json:"target"` + Error json.RawMessage `json:"error"` } // ParseOutput parses JSONL formatted dependency graph output. @@ -47,6 +48,7 @@ func (j *JSONLOutputParser) ParseOutput(data []byte) ([]DepGraphOutput, error) { TargetFileFromPlugin: parsed.TargetFileFromPlugin, Target: parsed.Target, DepGraph: parsed.DepGraph, + Error: parsed.Error, }) } diff --git a/pkg/depgraph/sbom_resolution.go b/pkg/depgraph/sbom_resolution.go index b9098a0..80211a9 100644 --- a/pkg/depgraph/sbom_resolution.go +++ b/pkg/depgraph/sbom_resolution.go @@ -136,6 +136,8 @@ func handleSBOMResolutionDI( workflowData = append(workflowData, data) } + totalFindings := len(findings) + if len(findings) == 0 || allProjects { applyFindingsExclusions(config, findings) @@ -143,12 +145,21 @@ func handleSBOMResolutionDI( if err != nil { return nil, err } - if legacyData != nil { - workflowData = append(workflowData, legacyData...) - } + + legacyWorkflowData, legacyProblemFindings := processLegacyData(logger, legacyData) + workflowData = append(workflowData, legacyWorkflowData...) + problemFindings = append(problemFindings, legacyProblemFindings...) + + totalFindings += len(legacyData) } - outputAnyWarnings(ctx, logger, problemFindings) + // TODO: This is a temporary implementation for rendering warnings. + // The long-term plan is for the CLI to handle all warning rendering. + // This will require extensions to handle `workflow.Data` objects with + // errors and propagate them upstream rather than rendering them directly. + // This change will require coordinated updates across extensions to + // ensure backwards compatibility and avoid breakages. + outputAnyWarnings(ctx, logger, problemFindings, totalFindings) return workflowData, nil } @@ -162,9 +173,9 @@ func logFindingError(logger *zerolog.Logger, lockFile string, err error) { } } -func outputAnyWarnings(ctx workflow.InvocationContext, logger *zerolog.Logger, problemFindings []scaplugin.Finding) { +func outputAnyWarnings(ctx workflow.InvocationContext, logger *zerolog.Logger, problemFindings []scaplugin.Finding, totalFindings int) { if len(problemFindings) > 0 { - message := renderWarningForProblemFindings(problemFindings) + message := renderWarningForProblemFindings(problemFindings, totalFindings) err := ctx.GetUserInterface().Output(message + "\n") if err != nil { @@ -173,7 +184,7 @@ func outputAnyWarnings(ctx workflow.InvocationContext, logger *zerolog.Logger, p } } -func renderWarningForProblemFindings(problemFindings []scaplugin.Finding) string { +func renderWarningForProblemFindings(problemFindings []scaplugin.Finding, totalFindings int) string { outputMessage := "" for _, finding := range problemFindings { outputMessage += fmt.Sprintf("\n%s:", finding.LockFile) @@ -184,12 +195,47 @@ func renderWarningForProblemFindings(problemFindings []scaplugin.Finding) string outputMessage += "\n could not process manifest file" } } - outputMessage += fmt.Sprintf("\n✗ %d potential projects failed to get dependencies.", len(problemFindings)) + outputMessage += fmt.Sprintf("\n✗ %d/%d potential projects failed to get dependencies.", len(problemFindings), totalFindings) redStyle := lipgloss.NewStyle().Foreground(lipgloss.AdaptiveColor{Light: "9", Dark: "1"}) return redStyle.Render(outputMessage) } +// processLegacyData separates successful dependency graphs from errors in the legacy data. +// It returns workflow data containing only valid dependency graphs, while converting +// any errors into problem findings that can be reported as warnings. +func processLegacyData(logger *zerolog.Logger, legacyData []workflow.Data) ([]workflow.Data, []scaplugin.Finding) { + workflowData := make([]workflow.Data, 0, len(legacyData)) + problemFindings := make([]scaplugin.Finding, 0) + + for _, data := range legacyData { + errList := data.GetErrorList() + if len(errList) > 0 { + problemFindings = append(problemFindings, extractProblemFindings(logger, data, errList)...) + continue + } + workflowData = append(workflowData, data) + } + + return workflowData, problemFindings +} + +func extractProblemFindings(logger *zerolog.Logger, data workflow.Data, errList []snyk_errors.Error) []scaplugin.Finding { + findings := make([]scaplugin.Finding, 0, len(errList)) + lockFile, metaErr := data.GetMetaData(contentLocationKey) + if metaErr != nil { + logger.Printf("Failed to get metadata %s for workflow data: %v", contentLocationKey, metaErr) + lockFile = "unknown" + } + for i := range errList { + findings = append(findings, scaplugin.Finding{ + LockFile: lockFile, + Error: errList[i], + }) + } + return findings +} + func getExclusionsFromFindings(findings []scaplugin.Finding) []string { exclusions := []string{} for i := range findings { @@ -238,7 +284,11 @@ func executeLegacyWorkflow( depGraphWorkflowFunc ResolutionHandlerFunc, findings []scaplugin.Finding, ) ([]workflow.Data, error) { - legacyData, err := depGraphWorkflowFunc(ctx, config, logger) + legacyConfig := config.Clone() + legacyConfig.Unset(FlagPrintEffectiveGraph) + legacyConfig.Set(FlagPrintEffectiveGraphWithErrors, true) + + legacyData, err := depGraphWorkflowFunc(ctx, legacyConfig, logger) if err == nil { return legacyData, nil } diff --git a/pkg/depgraph/sbom_resolution_test.go b/pkg/depgraph/sbom_resolution_test.go index eb0db67..7c9c373 100644 --- a/pkg/depgraph/sbom_resolution_test.go +++ b/pkg/depgraph/sbom_resolution_test.go @@ -881,6 +881,65 @@ func Test_callback_SBOMResolution(t *testing.T) { assert.Nil(t, mockPlugin.options.Exclude) }) + t.Run("should handle legacy workflow data with errors attached and output warnings", func(t *testing.T) { + ctx := setupTestContext(t, false) + ctx.config.Set(FlagAllProjects, true) + + // Create mock plugin that returns no findings to trigger legacy workflow + mockPlugin := &mockScaPlugin{ + findings: []scaplugin.Finding{}, + } + + // Create workflow data with an error attached + dataIdentifier := workflow.NewTypeIdentifier(WorkflowID, workflowIDStr) + validData := workflow.NewData( + dataIdentifier, + "application/json", + []byte(`{}`), + ) + + errorData := workflow.NewData( + dataIdentifier, + "application/json", + []byte(`{}`), + ) + errorData.SetMetaData(contentLocationKey, "legacy-project/requirements.txt") + errorData.AddError(snyk_errors.Error{ + ID: "SNYK-LEGACY-001", + Title: "Legacy Error Title", + Detail: "Detailed legacy error information for debugging", + }) + + resolutionHandler := NewCalledResolutionHandlerFunc([]workflow.Data{validData, errorData}, nil) + + // Capture the output to verify it contains expected error messages + var capturedOutput string + ctx.userInterface.EXPECT().Output(gomock.Any()).DoAndReturn(func(msg string) error { + capturedOutput = msg + return nil + }).Times(1) + + workflowData, err := handleSBOMResolutionDI( + ctx.invocationContext, + ctx.config, + &nopLogger, + []scaplugin.SCAPlugin{mockPlugin}, + resolutionHandler.Func(), + ) + + require.NoError(t, err) + assert.NotNil(t, workflowData) + assert.Len(t, workflowData, 1, "Should return only the valid data, filtering out error data") + assert.True(t, resolutionHandler.Called, "ResolutionHandlerFunc should be called") + + // Verify that the warning output contains the legacy error details + assert.Contains(t, capturedOutput, "legacy-project/requirements.txt", "Output should mention the problem file") + assert.Contains(t, capturedOutput, "Detailed legacy error information for debugging", + "Output should include snyk_errors.Error Detail field from legacy data") + assert.Contains(t, capturedOutput, "1/2 potential projects failed to get dependencies", + "Output should include number of failed potential projects") + }) + t.Run("should return snyk_errors.Error when finding has error and allProjects is false", func(t *testing.T) { ctx := setupTestContext(t, true) resolutionHandler := NewCalledResolutionHandlerFunc(nil, nil) @@ -1214,7 +1273,7 @@ func Test_callback_SBOMResolution(t *testing.T) { "Output should include snyk_errors.Error Detail field") assert.Contains(t, capturedOutput, "could not process manifest file", "Output should output a generic message rather than the error details of a non-snyk error") - assert.Contains(t, capturedOutput, "2 potential projects failed to get dependencies", + assert.Contains(t, capturedOutput, "2/4 potential projects failed to get dependencies", "Output should include number of failed potential projects") }) } @@ -1270,6 +1329,56 @@ func Test_parseExcludeFlag(t *testing.T) { } } +func Test_extractProblemFindings(t *testing.T) { + t.Run("should return findings with lockFile from metadata", func(t *testing.T) { + dataIdentifier := workflow.NewTypeIdentifier(WorkflowID, workflowIDStr) + data := workflow.NewData(dataIdentifier, "application/json", []byte(`{}`)) + data.SetMetaData(contentLocationKey, "project/requirements.txt") + + errList := []snyk_errors.Error{ + {ID: "SNYK-001", Title: "Error 1"}, + {ID: "SNYK-002", Title: "Error 2"}, + } + + findings := extractProblemFindings(&nopLogger, data, errList) + + require.Len(t, findings, 2) + assert.Equal(t, "project/requirements.txt", findings[0].LockFile) + assert.Equal(t, "project/requirements.txt", findings[1].LockFile) + assert.Equal(t, errList[0], findings[0].Error) + assert.Equal(t, errList[1], findings[1].Error) + }) + + t.Run("should use 'unknown' as lockFile when metadata retrieval fails", func(t *testing.T) { + dataIdentifier := workflow.NewTypeIdentifier(WorkflowID, workflowIDStr) + data := workflow.NewData(dataIdentifier, "application/json", []byte(`{}`)) + // Intentionally NOT setting metadata for contentLocationKey + + errList := []snyk_errors.Error{ + {ID: "SNYK-001", Title: "Error 1"}, + } + + findings := extractProblemFindings(&nopLogger, data, errList) + + require.Len(t, findings, 1) + assert.Equal(t, "unknown", findings[0].LockFile) + assert.Equal(t, errList[0], findings[0].Error) + }) + + t.Run("should return empty findings when errList is empty", func(t *testing.T) { + dataIdentifier := workflow.NewTypeIdentifier(WorkflowID, workflowIDStr) + data := workflow.NewData(dataIdentifier, "application/json", []byte(`{}`)) + data.SetMetaData(contentLocationKey, "project/requirements.txt") + + errList := []snyk_errors.Error{} + + findings := extractProblemFindings(&nopLogger, data, errList) + + assert.Len(t, findings, 0) + assert.NotNil(t, findings) + }) +} + func Test_getExclusionsFromFindings(t *testing.T) { testCases := []struct { name string diff --git a/pkg/depgraph/testdata/jsonl_dep_graph_with_error_output b/pkg/depgraph/testdata/jsonl_dep_graph_with_error_output new file mode 100644 index 0000000..5d1d4ee --- /dev/null +++ b/pkg/depgraph/testdata/jsonl_dep_graph_with_error_output @@ -0,0 +1,2 @@ +{"depGraph":{ "schemaVersion": "1.2.0", "pkgManager": { "name": "npm" }, "pkgs": [ { "id": "goof@1.0.1", "info": { "name": "goof", "version": "1.0.1" } } ], "graph": { "rootNodeId": "root-node", "nodes": [ { "nodeId": "root-node", "pkgId": "goof@1.0.1", "deps": [ { "nodeId": "adm-zip@0.4.7" }, { "nodeId": "body-parser@1.9.0" } ] } ] } },"normalisedTargetFile": "some normalised target file","targetFileFromPlugin": "some target file from plugin","target":{"key":"some target value"}} +{"error":{"jsonapi":{"version":"1.0"},"errors":[{"id":"d64b66ef-f5bd-4c8a-8e4b-983924945279","links":{"about":"https://docs.snyk.io/scan-with-snyk/error-catalog#snyk-cli-0000"},"status":"200","code":"SNYK-CLI-0000","title":"Unspecified Error","detail":"Something went wrong","meta":{"links":["https://docs.snyk.io/snyk-cli/commands","https://docs.snyk.io/snyk-cli/debugging-the-snyk-cli"],"isErrorCatalogError":true,"classification":"UNEXPECTED","level":"error"}}]},"normalisedTargetFile":"some normalised target file"}