Skip to content

Commit

Permalink
fix: severity filters when delta is enabled (#676)
Browse files Browse the repository at this point in the history
  • Loading branch information
ShawkyZ authored Oct 1, 2024
1 parent 7cf89ed commit bfe5202
Show file tree
Hide file tree
Showing 5 changed files with 109 additions and 23 deletions.
7 changes: 5 additions & 2 deletions application/server/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package server

import (
"context"
"github.com/snyk/snyk-ls/internal/product"
"os"
"reflect"
"strconv"
Expand Down Expand Up @@ -395,8 +396,10 @@ func updateSeverityFilter(c *config.Config, s types.SeverityFilter) {
}

for _, folder := range ws.Folders() {
// Nil means send for all products found in reported issues.
folder.FilterAndPublishDiagnostics(nil)
folder.FilterAndPublishDiagnostics(product.ProductOpenSource)
folder.FilterAndPublishDiagnostics(product.ProductInfrastructureAsCode)
folder.FilterAndPublishDiagnostics(product.ProductCode)
folder.FilterAndPublishDiagnostics(product.ProductContainer)
}
}
}
38 changes: 17 additions & 21 deletions domain/ide/workspace/folder.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ func (f *Folder) processResults(scanData snyk.ScanData) {
go sendAnalytics(&scanData)

// Filter and publish cached diagnostics
f.FilterAndPublishDiagnostics(&scanData.Product)
f.FilterAndPublishDiagnostics(scanData.Product)
}

func (f *Folder) sendScanError(product product.Product, err error) {
Expand Down Expand Up @@ -479,46 +479,42 @@ func appendTestResults(sic snyk.SeverityIssueCounts, results []json_schemas.Test
return results
}

func (f *Folder) FilterAndPublishDiagnostics(p *product.Product) {
productIssuesByFile, err := f.getDelta(f.IssuesByProduct(), p)
func (f *Folder) FilterAndPublishDiagnostics(p product.Product) {
issueByProduct := f.IssuesByProduct()
if len(issueByProduct[p]) == 0 {
return
}

productIssuesByFile, err := f.getDelta(issueByProduct, p)
if err != nil {
// Error can only be returned from delta analysis. Other non delta scans are skipped with no errors.
deltaErr := fmt.Errorf("couldn't determine the difference between current and base branch for %s scan. %w", p.ToProductNamesString(), err)
f.sendScanError(*p, deltaErr)
f.sendScanError(p, deltaErr)
return
}
if p != nil {
filteredIssues := f.filterDiagnostics(productIssuesByFile[*p])
f.publishDiagnostics(*p, filteredIssues)
} else {
for p, pIssues := range productIssuesByFile {
filteredIssues := f.filterDiagnostics(pIssues)
f.publishDiagnostics(p, filteredIssues)
}
}
filteredIssues := f.filterDiagnostics(productIssuesByFile[p])
f.publishDiagnostics(p, filteredIssues)
}

// Error can only be returned from delta analysis. Other non delta scans are skipped with no errors.
func (f *Folder) getDelta(productIssueByFile snyk.ProductIssuesByFile, p *product.Product) (snyk.ProductIssuesByFile, error) {
func (f *Folder) getDelta(productIssueByFile snyk.ProductIssuesByFile, p product.Product) (snyk.ProductIssuesByFile, error) {
logger := f.c.Logger().With().Str("method", "getDelta").Logger()
currentProduct := *p

if !f.c.IsDeltaFindingsEnabled() {
return productIssueByFile, nil
}

if len(productIssueByFile[currentProduct]) == 0 {
if len(productIssueByFile[p]) == 0 {
// If no issues found in current branch scan. We can't have deltas.
return productIssueByFile, nil
}

baseIssueList, err := f.scanPersister.GetPersistedIssueList(f.path, currentProduct)
baseIssueList, err := f.scanPersister.GetPersistedIssueList(f.path, p)
if err != nil {
logger.Err(err).Msg("Error getting persisted issue list")
return nil, delta.ErrNoDeltaCalculated
}

currentFlatIssueList := getFlatIssueList(productIssueByFile, currentProduct)
currentFlatIssueList := getFlatIssueList(productIssueByFile, p)
baseFindingIdentifiable := make([]delta.Identifiable, len(baseIssueList))
for i := range baseIssueList {
baseFindingIdentifiable[i] = &baseIssueList[i]
Expand All @@ -528,7 +524,7 @@ func (f *Folder) getDelta(productIssueByFile snyk.ProductIssuesByFile, p *produc
currentFindingIdentifiable[i] = &currentFlatIssueList[i]
}

df := delta2.NewDeltaFinderForProduct(currentProduct)
df := delta2.NewDeltaFinderForProduct(p)
diff, err := df.Diff(baseFindingIdentifiable, currentFindingIdentifiable)

if err != nil {
Expand All @@ -540,7 +536,7 @@ func (f *Folder) getDelta(productIssueByFile snyk.ProductIssuesByFile, p *produc
for i := range diff {
deltaSnykIssues[i] = *diff[i].(*snyk.Issue)
}
productIssueByFile[currentProduct] = getIssuePerFileFromFlatList(deltaSnykIssues)
productIssueByFile[p] = getIssuePerFileFromFlatList(deltaSnykIssues)

return productIssueByFile, nil
}
Expand Down
1 change: 1 addition & 0 deletions infrastructure/code/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ func (s *SarifConverter) toIssues(baseDir string) (issues []snyk.Issue, err erro
CWEs: testRule.Properties.Cwe,
}
d.SetFingerPrint(result.Fingerprints.Num1)
d.SetGlobalIdentity(result.Fingerprints.Identity)
d.IsIgnored, d.IgnoreDetails = s.getIgnoreDetails(result)
d.AdditionalData = additionalData

Expand Down
54 changes: 54 additions & 0 deletions internal/delta/delta_finder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package delta

import (
"github.com/google/uuid"
"golang.org/x/exp/slices"
"testing"

Expand Down Expand Up @@ -160,3 +161,56 @@ func TestFind_DifferWithEnricherWithMatcher_NoNewIssues(t *testing.T) {
assert.NotEmpty(t, enriched.GetGlobalIdentity())
}
}

func TestFind_DifferWithEnricherWithMatcher_NewIssue_ExistingId(t *testing.T) {
f := NewFinder(
WithEnricher(FindingsEnricher{}),
WithMatcher(FuzzyMatcher{}),
WithDiffer(FindingsDiffer{}),
)

baseIssueList := []mockIdentifiable{
{
ruleId: "javascript/UseCsurfForExpress",
startLine: 30,
endLine: 30,
startColumn: 10,
endColumn: 17,
path: "/var/folders/qt/rlk4r6d55s1fx7bdr7bg0w3h0000gn/T/snyk_tmp_repo2525628625/app.js",
fingerprint: "ae77ea27.4773f344.607187b5.d7919eeb.a1fb1152.5fce695c.fee35010.89d75565.630e4ed1.4773f344.aa4dda5f.d7919eeb.f30fb760.49b28873.85bdc101.83642794",
},
{
ruleId: "javascript/NoHardcodedPasswords",
startLine: 40,
endLine: 40,
startColumn: 10,
endColumn: 17,
path: "/var/folders/qt/rlk4r6d55s1fx7bdr7bg0w3h0000gn/T/snyk_tmp_repo2525628625/db.js",
fingerprint: "12567ef6.6d936dbf.bd65d204.fd94bb7c.79a7d027.fcf3002d.81d021f5.91c60b7d.12567ef6.6d936dbf.bd65d204.fd94bb7c.79a7d027.fcf3002d.81d021f5.91c60b7d",
},
}
existingIdentity := uuid.New().String()
newIssue := mockIdentifiable{
ruleId: "javascript/NoHardcodedPasswords",
startLine: 10,
endLine: 50,
startColumn: 10,
endColumn: 17,
path: "/var/folders/qt/rlk4r6d55s1fx7bdr7bg0w3h0000gn/T/snyk_tmp_repo2525628625/newfile.js",
fingerprint: "1256723f6.6d16dbf.bd25d204.fd9wwb7c.79aff027.fcf30ddd.81d021ss.91c60baad.12567cf6.6d9cc6dbf.bd6cs204.fd94cc7c.79ss027.fcs002d.8dd021f5.91c6ss7d",
globalIdentity: existingIdentity,
}

currentIssueList := slices.Clone(baseIssueList)
currentIssueList = append(currentIssueList, newIssue)
baseFindingIdentifiable := convertToFindingsIdentifiable(baseIssueList)
currentFindingIdentifiable := convertToFindingsIdentifiable(currentIssueList)
enrichedList, err := f.Enrich(baseFindingIdentifiable, currentFindingIdentifiable)

assert.NoError(t, err)
assert.Len(t, enrichedList, 3)

assert.True(t, enrichedList[2].GetIsNew())
assert.Equal(t, enrichedList[2].GetGlobalIdentity(), currentIssueList[2].GetGlobalIdentity())
assert.Equal(t, existingIdentity, enrichedList[2].GetGlobalIdentity())
}
32 changes: 32 additions & 0 deletions internal/delta/fuzzy_matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package delta

import (
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"golang.org/x/exp/slices"
"testing"
Expand Down Expand Up @@ -50,6 +51,37 @@ func Test_New_Issue(t *testing.T) {
assert.Equal(t, newIssue.GetFingerprint(), finding.GetFingerprint())
}

func Test_New_Issue_No_IdChange(t *testing.T) {
baseIssueList := getIssueList()

exisingIdentity := uuid.New().String()
newIssue := mockIdentifiable{
ruleId: "javascript/NoHardcodedPasswords",
startLine: 10,
endLine: 50,
startColumn: 10,
endColumn: 17,
path: "/var/folders/qt/rlk4r6d55s1fx7bdr7bg0w3h0000gn/T/snyk_tmp_repo2525628625/newfile.js",
fingerprint: "1256723f6.6d16dbf.bd25d204.fd9wwb7c.79aff027.fcf30ddd.81d021ss.91c60baad.12567cf6.6d9cc6dbf.bd6cs204.fd94cc7c.79ss027.fcs002d.8dd021f5.91c6ss7d",
globalIdentity: exisingIdentity,
}
df := initDeltaFinder()

currentIssueList := slices.Clone(baseIssueList)
currentIssueList = append(currentIssueList, newIssue)

baseFindingIdentifiable := convertToFindingsIdentifiable(baseIssueList)
currentFindingIdentifiable := convertToFindingsIdentifiable(currentIssueList)

deltaList, err := df.Diff(baseFindingIdentifiable, currentFindingIdentifiable)
assert.NoError(t, err)
assert.Equal(t, 1, len(deltaList))
finding, ok := deltaList[0].(Fingerprintable)
assert.True(t, ok)
assert.Equal(t, newIssue.GetFingerprint(), finding.GetFingerprint())
assert.Equal(t, exisingIdentity, newIssue.GetGlobalIdentity())
}

func Test_No_New_Issue(t *testing.T) {
baseIssueList := getIssueList()
currentIssueList := getIssueList()
Expand Down

0 comments on commit bfe5202

Please sign in to comment.