Skip to content

feat: DGP-789 - group text output by Open Issues and License issues #47

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: DGP-789 - group text output by Open Issues and License issues #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions internal/presenters/__snapshots__/presenter_unified_finding_test.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
[TestUnifiedFindingPresenter_CliOutput/snapshot_test_of_human-readable_output - 1]

Testing ...

Open issues: 1

✗ [HIGH] High severity vulnerability
Finding ID: SNYK-JS-VM2-5537100
Info: https://snyk.io/vuln/SNYK-JS-VM2-5537100
Risk Score: 780


License issues: 1

✗ [MEDIUM] LGPL-3.0 license
Finding ID: snyk:lic:npm:web3-core:LGPL-3.0
Info: https://snyk.io/vuln/snyk:lic:npm:web3-core:LGPL-3.0


╭────────────────────────────────────────────────────────────────╮
│ Test Summary │
│ │
│ Organization: │
│ Test type: open-source │
│ Project path: │
│ │
│ Total issues: 2 │
│ Ignored issues: 0 [ 0 CRITICAL 0 HIGH 0 MEDIUM 0 LOW ] │
│ Open issues: 2 [ 0 CRITICAL 1 HIGH 1 MEDIUM 0 LOW ] │
╰────────────────────────────────────────────────────────────────╯
💡 Tip

To view ignored issues, use the --include-ignores option.


---
54 changes: 39 additions & 15 deletions internal/presenters/funcs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,42 @@ func isIgnoredFinding() func(obj any) bool {
}
}

// isLicenseFinding returns true if the finding is a license finding.
func isLicenseFinding(finding testapi.FindingData) bool {
if finding.Attributes != nil {
for _, problem := range finding.Attributes.Problems {
disc, err := problem.Discriminator()
if err == nil && disc == string(testapi.SnykLicense) {
return true
}
}
}
return false
}

// isLicenseFindingFilter returns a filter function that checks if a finding is a license finding.
func isLicenseFindingFilter() func(obj any) bool {
return func(obj any) bool {
finding, ok := obj.(testapi.FindingData)
if !ok {
return false
}
return isLicenseFinding(finding)
}
}

// isNotLicenseFindingFilter returns a function that checks if a finding is not a license finding.
func isNotLicenseFindingFilter() func(obj any) bool {
return func(obj any) bool {
finding, ok := obj.(testapi.FindingData)
if !ok {
return true
}
isLicense := isLicenseFinding(finding)
return !isLicense
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Inconsistent Filter Behavior

The isNotLicenseFindingFilter function returns true when the input object is not a testapi.FindingData. This is inconsistent with other similar filter functions (e.g., isOpenFinding, isPendingFinding, isIgnoredFinding, isLicenseFindingFilter), which return false in the same scenario. Consequently, non-FindingData objects are incorrectly included in the "not license findings" filter results.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is by design. Licenses are called out as special, and everything else falls under "Other issues."


// hasSuppression checks if a finding has any suppression.
func hasSuppression(finding testapi.FindingData) bool {
if finding.Attributes.Suppression == nil {
Expand All @@ -299,7 +335,8 @@ func getCliTemplateFuncMap(tmpl *template.Template) template.FuncMap {
fnMap["divider"] = RenderDivider
fnMap["title"] = RenderTitle
fnMap["renderToString"] = renderTemplateToString(tmpl)
fnMap["isLicenseFinding"] = isLicenseFinding
fnMap["isLicenseFindingFilter"] = isLicenseFindingFilter
fnMap["isNotLicenseFindingFilter"] = isNotLicenseFindingFilter
fnMap["isOpenFinding"] = isOpenFinding
fnMap["isPendingFinding"] = isPendingFinding
fnMap["isIgnoredFinding"] = isIgnoredFinding
Expand Down Expand Up @@ -366,26 +403,13 @@ func getDefaultTemplateFuncMap(config configuration.Configuration, ri runtimeinf
defaultMap["formatDatetime"] = formatDatetime
defaultMap["getSourceLocation"] = getSourceLocation
defaultMap["getFindingId"] = getFindingID
defaultMap["hasPrefix"] = strings.HasPrefix
defaultMap["isLicenseFinding"] = isLicenseFinding
defaultMap["hasPrefix"] = strings.HasPrefix
defaultMap["constructDisplayPath"] = constructDisplayPath(config)

return defaultMap
}

// isLicenseFinding returns true if the finding is a license finding.
func isLicenseFinding(finding testapi.FindingData) bool {
if finding.Attributes != nil {
for _, problem := range finding.Attributes.Problems {
disc, err := problem.Discriminator()
if err == nil && disc == string(testapi.SnykLicense) {
return true
}
}
}
return false
}

// reverse reverses the order of elements in a slice.
func reverse(v interface{}) []interface{} {
l, err := mustReverse(v)
Expand Down
102 changes: 99 additions & 3 deletions internal/presenters/presenter_unified_finding_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import (
"bytes"
"testing"

"github.com/charmbracelet/lipgloss"
"github.com/gkampitakis/go-snaps/snaps"
"github.com/google/uuid"
"github.com/muesli/termenv"
"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/json_schemas"
Expand Down Expand Up @@ -32,7 +35,7 @@ func TestJsonWriter(t *testing.T) {
assert.Equal(t, expected, buffer.String())
})

t.Run("Don't strip whitespaces while writing", func(t *testing.T) {
t.Run("don't strip whitespaces while writing", func(t *testing.T) {
buffer := &bytes.Buffer{}
writerUnderTest := presenters.NewJSONWriter(buffer, false)

Expand Down Expand Up @@ -78,7 +81,9 @@ func TestUnifiedFindingPresenter_CliOutput(t *testing.T) {
projectResult := &presenters.UnifiedProjectResult{
Findings: []testapi.FindingData{licenseFinding},
Summary: &json_schemas.TestSummary{
SeverityOrderAsc: []string{"critical", "high", "medium", "low", "none"},
Type: "open-source",
Path: "test/path",
SeverityOrderAsc: []string{"low", "medium", "high", "critical"},
Results: []json_schemas.TestSummaryResult{
{
Severity: "medium",
Expand Down Expand Up @@ -128,7 +133,9 @@ func TestUnifiedFindingPresenter_CliOutput(t *testing.T) {
projectResult := &presenters.UnifiedProjectResult{
Findings: []testapi.FindingData{vulnFinding},
Summary: &json_schemas.TestSummary{
SeverityOrderAsc: []string{"critical", "high", "medium", "low", "none"},
Type: "open-source",
Path: "test/path",
SeverityOrderAsc: []string{"low", "medium", "high", "critical"},
Results: []json_schemas.TestSummaryResult{
{
Severity: "high",
Expand All @@ -152,4 +159,93 @@ func TestUnifiedFindingPresenter_CliOutput(t *testing.T) {
output := buffer.String()
assert.Contains(t, output, "Risk Score: 780")
})

t.Run("snapshot test of human-readable output", func(t *testing.T) {
// setup
config := configuration.New()
buffer := &bytes.Buffer{}
lipgloss.SetColorProfile(termenv.Ascii)

riskScore := uint16(780)
problemID := "SNYK-JS-VM2-5537100"
vulnFinding := testapi.FindingData{
Id: util.Ptr(uuid.MustParse("22222222-2222-2222-2222-222222222222")),
Type: util.Ptr(testapi.Findings),
Attributes: &testapi.FindingAttributes{
Title: "High severity vulnerability",
Risk: testapi.Risk{
RiskScore: &testapi.RiskScore{
Value: riskScore,
},
},
Rating: testapi.Rating{
Severity: testapi.Severity("high"),
},
Problems: func() []testapi.Problem {
var p testapi.Problem
err := p.FromSnykVulnProblem(testapi.SnykVulnProblem{
Id: problemID,
Source: testapi.SnykVuln,
})
assert.NoError(t, err)
return []testapi.Problem{p}
}(),
},
}

licProblemID := "snyk:lic:npm:web3-core:LGPL-3.0"
licenseFinding := testapi.FindingData{
Id: util.Ptr(uuid.MustParse("33333333-3333-3333-3333-333333333333")),
Type: util.Ptr(testapi.Findings),
Attributes: &testapi.FindingAttributes{
Title: "LGPL-3.0 license",
Rating: testapi.Rating{
Severity: testapi.Severity("medium"),
},
Problems: func() []testapi.Problem {
var p testapi.Problem
err := p.FromSnykLicenseProblem(testapi.SnykLicenseProblem{
Id: licProblemID,
License: string(testapi.SnykLicense),
})
assert.NoError(t, err)
return []testapi.Problem{p}
}(),
},
}

projectResult := &presenters.UnifiedProjectResult{
Findings: []testapi.FindingData{vulnFinding, licenseFinding},
Summary: &json_schemas.TestSummary{
Type: "open-source",
Path: "test/path",
SeverityOrderAsc: []string{"low", "medium", "high", "critical"},
Results: []json_schemas.TestSummaryResult{
{
Severity: "high",
Open: 1,
Total: 1,
},
{
Severity: "medium",
Open: 1,
Total: 1,
},
},
},
}

presenter := presenters.NewUnifiedFindingsRenderer(
[]*presenters.UnifiedProjectResult{projectResult},
config,
buffer,
)

// execute
err := presenter.RenderTemplate(presenters.DefaultTemplateFiles, presenters.DefaultMimeType)

// assert
assert.NoError(t, err)
snaps.MatchSnapshot(t, buffer.String())
})
}
21 changes: 18 additions & 3 deletions internal/presenters/templates/unified_finding.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,16 @@
{{- with (getIntroducedThrough .) }}
Introduced through: {{ . }}
{{- end }}

{{- if not (isLicenseFinding .) }}
{{- $riskScore := getFieldValueFrom . "Attributes.Risk.RiskScore.Value" -}}
{{- $riskScore := getFieldValueFrom . "Attributes.Risk.RiskScore.Value" }}
{{- if $riskScore }}
Risk Score: {{ $riskScore }}
{{- else }}
Risk Score: N/A
{{- end }}
{{- end }}

{{- $reachability := getReachability . -}}
{{- if ne $reachability "N/A" }}
Reachability: {{ $reachability }}
Expand All @@ -26,20 +28,33 @@

{{- define "details" -}}
{{- $sortedFindings := .Findings | sortFindingBy "Attributes.Rating.Severity" .Summary.SeverityOrderAsc }}
{{- $openFindings := $sortedFindings | filterFinding (isOpenFinding) }}
{{- $allOpenFindings := $sortedFindings | filterFinding (isOpenFinding) }}
{{- $openFindings := $allOpenFindings | filterFinding (isNotLicenseFindingFilter) }}
{{- $licenseFindings := $allOpenFindings | filterFinding (isLicenseFindingFilter) }}
{{- $pendingIgnoreFindings := $sortedFindings | filterFinding (isPendingFinding) }}
{{- $ignoredFindings := $sortedFindings | filterFinding (isIgnoredFinding) }}
{{- $hasOpenFindings := gt (len $openFindings) 0 }}
{{- $hasLicenseFindings := gt (len $licenseFindings) 0 }}
{{- $hasPendingIgnoreFindings := gt (len $pendingIgnoreFindings) 0 }}
{{- $hasIgnoredFindings := gt (len $ignoredFindings) 0 }}
{{- if $hasOpenFindings }}{{ "Open Issues" | title }}

{{- if $hasOpenFindings }}{{ printf "Open issues: %d" (len $openFindings) | title }}
{{- range $finding := $openFindings }}
{{- $severity := getFieldValueFrom $finding "Attributes.Rating.Severity" -}}
{{- $title := getFieldValueFrom $finding "Attributes.Title" -}}
{{- printf " ✗ %s %s" (printf "[%s]" ($severity | toUpperCase) | renderInSeverityColor) ($title | bold) -}}
{{- template "finding_details" $finding -}}
{{- end }}
{{- end }}

{{- if $hasLicenseFindings }}{{ printf "License issues: %d" (len $licenseFindings) | title }}
{{- range $finding := $licenseFindings }}
{{- $severity := getFieldValueFrom $finding "Attributes.Rating.Severity" -}}
{{- $title := getFieldValueFrom $finding "Attributes.Title" -}}
{{- printf " ✗ %s %s" (printf "[%s]" ($severity | toUpperCase) | renderInSeverityColor) ($title | bold) -}}
{{- template "finding_details" $finding -}}
{{- end }}
{{- end }}
{{- if $hasPendingIgnoreFindings }}
{{- range $finding := $pendingIgnoreFindings }}
{{- $severity := getFieldValueFrom $finding "Attributes.Rating.Severity" -}}
Expand Down