diff --git a/ast/ast.go b/ast/ast.go index 39172bc8f..bb12690b9 100644 --- a/ast/ast.go +++ b/ast/ast.go @@ -16,21 +16,34 @@ package ast +import "fmt" + type Node struct { - Line int - StartChar int - EndChar int - DocOffset int64 - Parent *Node - Children []*Node - Name string - Value string - Attributes map[string]string + Line int + StartChar int + EndChar int + DocOffset int64 + Parent *Node + Children []*Node + Name string + Value string + Attributes map[string]string + Tree *Tree + LinkedParentDependencyNode *Node } type Tree struct { - Root *Node - Document string + ParentTree *Tree + Root *Node + Document string +} + +func (t *Tree) String() string { + return fmt.Sprintf("Root=%s, Document=%s, ParentTree=%s", t.Root, t.Document, t.ParentTree) +} + +func (t *Tree) DebugString() string { + return t.String() } type Parser interface { @@ -45,6 +58,14 @@ func (n *Node) Accept(v Visitor) { v.visit(n) } +func (n *Node) String() string { + return fmt.Sprintf("Name=%s, Value=%s, Position=%d:%d:%d, Tree=%s, Parent=%s, LinkedParentDependencyNode=%s", n.Name, n.Value, n.Line, n.StartChar, n.EndChar, n.Tree, n.Parent, n.LinkedParentDependencyNode) +} + +func (n *Node) DebugString() string { + return fmt.Sprintf("%s, DocOffset: %d, ChildrenCount: %d", n.String(), n.DocOffset, len(n.Children)) +} + func (n *Node) Add(child *Node) *Node { n.Children = append(n.Children, child) child.Parent = n diff --git a/ast/maven/parser.go b/ast/maven/parser.go index dac11ddf5..a6fd8c700 100644 --- a/ast/maven/parser.go +++ b/ast/maven/parser.go @@ -20,6 +20,8 @@ import ( "encoding/xml" "errors" "io" + "os" + "path/filepath" "strings" "github.com/snyk/snyk-ls/application/config" @@ -27,15 +29,22 @@ import ( ) type Parser struct { - tree ast.Tree config *config.Config } -type dependency struct { +type Parent struct { + Group string `xml:"group"` + ArtifactId string `xml:"artifactId"` + Version string `xml:"version"` + RelativePath string `xml:"relativePath"` +} + +type Dependency struct { Group string `xml:"group"` ArtifactId string `xml:"artifactId"` Version string `xml:"version"` Scope string `xml:"scope"` + Type string `xml:"type"` } func New(c *config.Config) Parser { @@ -44,10 +53,11 @@ func New(c *config.Config) Parser { } } -func (p *Parser) Parse(content string, path string) ast.Tree { +func (p *Parser) Parse(content string, path string) *ast.Tree { tree := p.initTree(path, content) d := xml.NewDecoder(strings.NewReader(content)) var offset int64 + pomDir := filepath.Dir(path) for { token, err := d.Token() offset = d.InputOffset() @@ -61,13 +71,44 @@ func (p *Parser) Parse(content string, path string) ast.Tree { switch xmlType := token.(type) { case xml.StartElement: if xmlType.Name.Local == "dependency" { - var dep dependency + var dep Dependency if err = d.DecodeElement(&dep, &xmlType); err != nil { - p.config.Logger().Err(err).Msg("Couldn't decode dependency") + p.config.Logger().Err(err).Msg("Couldn't decode Dependency") + continue } + + if strings.ToLower(dep.Type) == "bom" { + addDepsFromBOM(path, tree, dep) + } + offsetAfter := d.InputOffset() node := p.addNewNodeTo(tree.Root, offset, offsetAfter, dep) - p.config.Logger().Debug().Interface("nodeName", node.Name).Str("path", p.tree.Document).Msg("Added dependency node") + p.config.Logger().Debug().Interface("nodeName", node.Name).Str("path", tree.Document).Msg("Added Dependency node") + } + if xmlType.Name.Local == "parent" { + // parse Parent pom + var parentPOM Parent + if err = d.DecodeElement(&parentPOM, &xmlType); err != nil { + p.config.Logger().Err(err).Msg("Couldn't decode Parent") + continue + } + + if parentPOM.RelativePath == "" { + parentPOM.RelativePath = filepath.Join("..", "pom.xml") + } + + parentAbsPath, err := filepath.Abs(filepath.Join(pomDir, parentPOM.RelativePath)) + if err != nil { + p.config.Logger().Err(err).Msg("Couldn't resolve Parent path") + continue + } + content, err := os.ReadFile(parentAbsPath) + if err != nil { + p.config.Logger().Err(err).Msg("Couldn't read Parent file") + continue + } + parentTree := p.Parse(string(content), parentAbsPath) + tree.ParentTree = parentTree } default: } @@ -75,7 +116,11 @@ func (p *Parser) Parse(content string, path string) ast.Tree { return tree } -func (p *Parser) initTree(path string, content string) ast.Tree { +func addDepsFromBOM(path string, tree *ast.Tree, dep Dependency) { + // todo retrieve, potentially from configured repos (not parsed yet) +} + +func (p *Parser) initTree(path string, content string) *ast.Tree { var currentLine = 0 root := ast.Node{ Line: currentLine, @@ -87,35 +132,49 @@ func (p *Parser) initTree(path string, content string) ast.Tree { Name: path, Value: content, } - p.tree = ast.Tree{ + + root.Tree = &ast.Tree{ Root: &root, Document: path, } - return p.tree + return root.Tree } -func (p *Parser) addNewNodeTo(parent *ast.Node, offsetBefore int64, offsetAfter int64, dep dependency) *ast.Node { - content := p.tree.Root.Value - contentInclusive := content[0:offsetAfter] +func (p *Parser) addNewNodeTo(parent *ast.Node, offsetBefore int64, offsetAfter int64, dep Dependency) *ast.Node { + var startChar int + var endChar int + var line int + content := parent.Tree.Root.Value + contentInclusiveDep := content[0:offsetAfter] + startTag := "" - endTag := " + + + 4.0.0 + + com.mycompany.app + my-app + 1.0-SNAPSHOT + pom + + my-app + http://www.example.com + + + UTF-8 + 1.7 + 1.7 + + + + + + junit + junit + 4.11 + test + + + commons-fileupload + commons-fileupload + 1.2.1 + compile + + + + diff --git a/ast/maven/testdata/maven-goof/sub/pom.xml b/ast/maven/testdata/maven-goof/sub/pom.xml new file mode 100644 index 000000000..837434476 --- /dev/null +++ b/ast/maven/testdata/maven-goof/sub/pom.xml @@ -0,0 +1,85 @@ + + + + 4.0.0 + + com.mycompany.app + my-child-app + 1.0-SNAPSHOT + + + my-app + + http://www.example.com + + + com.mycompany.app + my-app + 1.0-SNAPSHOT + + + + + UTF-8 + 1.7 + 1.7 + + + + + junit + junit + + + commons-fileupload + commons-fileupload + + + + + + + + + maven-clean-plugin + 3.1.0 + + + + maven-resources-plugin + 3.0.2 + + + maven-compiler-plugin + 3.8.0 + + + maven-surefire-plugin + 2.22.1 + + + maven-jar-plugin + 3.0.2 + + + maven-install-plugin + 2.5.2 + + + maven-deploy-plugin + 2.8.2 + + + + maven-site-plugin + 3.7.1 + + + maven-project-info-reports-plugin + 3.0.0 + + + + + diff --git a/domain/ide/command/code_fix.go b/domain/ide/command/code_fix.go index 9d87e9785..060925382 100644 --- a/domain/ide/command/code_fix.go +++ b/domain/ide/command/code_fix.go @@ -54,41 +54,35 @@ func (cmd *fixCodeIssue) Execute(_ context.Context) (any, error) { if err != nil { return nil, errors.Join(err, fmt.Errorf("Failed to parse code action id.")) } - issuePath, ok := args[1].(string) - if !ok { - return nil, fmt.Errorf("Failed to parse issue path.") - } - issueRange, err := cmd.toRange(args[2]) - if err != nil { - return nil, errors.Join(err, fmt.Errorf("invalid range parameter")) - } - issues := cmd.issueProvider.IssuesForRange(issuePath, issueRange) - for i := range issues { - for _, action := range issues[i].CodeActions { - if action.Uuid == nil || *action.Uuid != codeActionId { - continue - } - - // execute autofix codeaction - edit := (*action.DeferredEdit)() - if edit == nil { - cmd.logger.Debug().Msg("No fix could be computed.") + issueMap := cmd.issueProvider.Issues() + for _, issues := range issueMap { + for i := range issues { + for _, action := range issues[i].CodeActions { + if action.Uuid == nil || *action.Uuid != codeActionId { + continue + } + + // execute autofix codeaction + edit := (*action.DeferredEdit)() + if edit == nil { + cmd.logger.Debug().Msg("No fix could be computed.") + return nil, nil + } + + cmd.notifier.Send(types.ApplyWorkspaceEditParams{ + Label: "Snyk Code fix", + Edit: converter.ToWorkspaceEdit(edit), + }) + + // reset codelenses + issues[i].CodelensCommands = nil + + // Give client some time to apply edit, then refresh code lenses to hide stale codelens for the fixed issue + time.Sleep(1 * time.Second) + cmd.notifier.Send(types.CodeLensRefresh{}) return nil, nil } - - cmd.notifier.Send(types.ApplyWorkspaceEditParams{ - Label: "Snyk Code fix", - Edit: converter.ToWorkspaceEdit(edit), - }) - - // reset codelenses - issues[i].CodelensCommands = nil - - // Give client some time to apply edit, then refresh code lenses to hide stale codelens for the fixed issue - time.Sleep(1 * time.Second) - cmd.notifier.Send(types.CodeLensRefresh{}) - return nil, nil } } diff --git a/domain/ide/command/code_fix_test.go b/domain/ide/command/code_fix_test.go index 09051fbca..71587d095 100644 --- a/domain/ide/command/code_fix_test.go +++ b/domain/ide/command/code_fix_test.go @@ -52,7 +52,8 @@ type issueProviderMock struct { } func (m *issueProviderMock) Issues() snyk.IssuesByFile { - panic("this should not be called") + args := m.Called() + return args.Get(0).(snyk.IssuesByFile) } func (m *issueProviderMock) Issue(_ string) snyk.Issue { @@ -153,9 +154,12 @@ func Test_fixCodeIssue_sendsSuccessfulEdit(t *testing.T) { DeferredEdit: &deferredMockEdit, } issues := setupSampleIssues(issueRange, codeAction, cmd.command) + issueMap := snyk.IssuesByFile{ + filePath.(string): issues, + } issueProviderMock := new(issueProviderMock) - issueProviderMock.On("IssuesForRange", filePath, issueRange).Return(issues) + issueProviderMock.On("Issues").Return(issueMap) cmd.issueProvider = issueProviderMock // act @@ -192,9 +196,12 @@ func Test_fixCodeIssue_noEdit(t *testing.T) { DeferredEdit: &deferredMockEdit, } issues := setupSampleIssues(issueRange, codeAction, cmd.command) + issueMap := snyk.IssuesByFile{ + filePath.(string): issues, + } issueProviderMock := new(issueProviderMock) - issueProviderMock.On("IssuesForRange", filePath, issueRange).Return(issues) + issueProviderMock.On("Issues").Return(issueMap) cmd.issueProvider = issueProviderMock // act @@ -218,14 +225,9 @@ func Test_fixCodeIssue_NoIssueFound(t *testing.T) { mockNotifier := notification.NewMockNotifier() cmd := setupCommand(mockNotifier) - filePath := sampleArgs[1] - rangeDto, ok := sampleArgs[2].(RangeDto) - require.True(t, ok) - issueRange, err := cmd.toRange(rangeDto) - require.NoError(t, err) - issueProviderMock := new(issueProviderMock) - issueProviderMock.On("IssuesForRange", filePath, issueRange).Return([]snyk.Issue{}) + issueProviderMock.On("Issues").Return(snyk.IssuesByFile{}) + cmd.issueProvider = issueProviderMock // act diff --git a/infrastructure/oss/cli_scanner.go b/infrastructure/oss/cli_scanner.go index 01e76daad..be638c1eb 100644 --- a/infrastructure/oss/cli_scanner.go +++ b/infrastructure/oss/cli_scanner.go @@ -490,7 +490,7 @@ func (cliScanner *CLIScanner) retrieveIssues( // we are updating the cli scanner maps/attributes in parallel, so we need to lock cliScanner.mutex.Lock() defer cliScanner.mutex.Unlock() - issues := convertScanResultToIssues(res, targetFilePath, fileContent, cliScanner.learnService, cliScanner.errorReporter, cliScanner.packageIssueCache) + issues := convertScanResultToIssues(cliScanner.config, res, targetFilePath, fileContent, cliScanner.learnService, cliScanner.errorReporter, cliScanner.packageIssueCache) // repopulate cliScanner.addVulnerabilityCountsToCache(issues) diff --git a/infrastructure/oss/code_actions.go b/infrastructure/oss/code_actions.go index 3867f8d1e..14682a742 100644 --- a/infrastructure/oss/code_actions.go +++ b/infrastructure/oss/code_actions.go @@ -18,27 +18,37 @@ package oss import ( "fmt" - "reflect" "strings" "github.com/pkg/errors" "golang.org/x/mod/semver" "github.com/snyk/snyk-ls/application/config" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/domain/snyk" "github.com/snyk/snyk-ls/infrastructure/learn" "github.com/snyk/snyk-ls/internal/observability/error_reporting" "github.com/snyk/snyk-ls/internal/types" ) -func (i *ossIssue) AddCodeActions(learnService learn.Service, ep error_reporting.ErrorReporter, affectedFilePath string, issueRange snyk.Range, fileContent []byte) (actions []snyk.CodeAction) { +func (i *ossIssue) AddCodeActions(learnService learn.Service, ep error_reporting.ErrorReporter, affectedFilePath string, issueDepNode *ast.Node, fileContent []byte) (actions []snyk.CodeAction) { c := config.CurrentConfig() - if reflect.DeepEqual(issueRange, snyk.Range{}) { - c.Logger().Debug().Str("issue", i.Id).Msg("skipping adding code action, as issueRange is empty") + if issueDepNode == nil { + c.Logger().Debug().Str("issue", i.Id).Msg("skipping adding code action, as issueDepNode is empty") return actions } - quickFixAction := i.AddQuickFixAction(affectedFilePath, issueRange, fileContent) + // let's see if we can offer a quickfix here + // value has the version information, so if it's empty, we'll need to look at the parent + var quickFixAction *snyk.CodeAction + if issueDepNode.Tree != nil && issueDepNode.Value == "" { + fixNode := issueDepNode.LinkedParentDependencyNode + if fixNode != nil { + quickFixAction = i.AddQuickFixAction(fixNode.Tree.Document, getRangeFromNode(fixNode), []byte(fixNode.Tree.Root.Value), true) + } + } else { + quickFixAction = i.AddQuickFixAction(affectedFilePath, getRangeFromNode(issueDepNode), fileContent, false) + } if quickFixAction != nil { actions = append(actions, *quickFixAction) } @@ -95,7 +105,7 @@ func (i *ossIssue) AddSnykLearnAction(learnService learn.Service, ep error_repor return action } -func (i *ossIssue) AddQuickFixAction(affectedFilePath string, issueRange snyk.Range, fileContent []byte) *snyk.CodeAction { +func (i *ossIssue) AddQuickFixAction(affectedFilePath string, issueRange snyk.Range, fileContent []byte, addFileNameToFixTitle bool) *snyk.CodeAction { logger := config.CurrentConfig().Logger().With().Str("method", "oss.AddQuickFixAction").Logger() if !config.CurrentConfig().IsSnykOSSQuickFixCodeActionsEnabled() { return nil @@ -106,6 +116,9 @@ func (i *ossIssue) AddQuickFixAction(affectedFilePath string, issueRange snyk.Ra return nil } upgradeMessage := "⚡️ Upgrade to " + quickfixEdit + if addFileNameToFixTitle { + upgradeMessage += " [ in file: " + affectedFilePath + " ]" + } autofixEditCallback := func() *snyk.WorkspaceEdit { edit := &snyk.WorkspaceEdit{} singleTextEdit := snyk.TextEdit{ diff --git a/infrastructure/oss/html_range_finder.go b/infrastructure/oss/html_range_finder.go index 6e2f68f5a..21b3a63aa 100644 --- a/infrastructure/oss/html_range_finder.go +++ b/infrastructure/oss/html_range_finder.go @@ -20,7 +20,7 @@ import ( "fmt" "github.com/snyk/snyk-ls/application/config" - "github.com/snyk/snyk-ls/domain/snyk" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/infrastructure/oss/parser" ) @@ -30,18 +30,23 @@ type htmlRangeFinder struct { config *config.Config } -func (h htmlRangeFinder) find(issue ossIssue) snyk.Range { +func (h htmlRangeFinder) find(introducingPackageName string, introducingVersion string) (*ast.Node, *ast.Tree) { dependencyParser := parser.NewParser(h.config, h.path) dependencies, err := dependencyParser.Parse(h.path) if err != nil { - return snyk.Range{} + return nil, nil } for _, dependency := range dependencies { - if fmt.Sprintf("%s@%s", dependency.ArtifactID, dependency.Version) == issue.From[0] { - return dependency.Range + format := "%s@%s" + if fmt.Sprintf(format, dependency.ArtifactID, dependency.Version) == fmt.Sprintf(format, introducingPackageName, introducingVersion) { + return &ast.Node{ + Line: dependency.Range.Start.Line, + StartChar: dependency.Range.Start.Character, + EndChar: dependency.Range.End.Character, + }, nil } } - return snyk.Range{} + return nil, nil } var _ RangeFinder = &htmlRangeFinder{} diff --git a/infrastructure/oss/issue.go b/infrastructure/oss/issue.go index 9f40818c3..70c5ac848 100644 --- a/infrastructure/oss/issue.go +++ b/infrastructure/oss/issue.go @@ -21,6 +21,7 @@ import ( "fmt" "strings" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/infrastructure/utils" "github.com/gomarkdown/markdown" @@ -40,9 +41,9 @@ var issuesSeverity = map[string]snyk.Severity{ "medium": snyk.Medium, } -func toIssue(affectedFilePath string, issue ossIssue, scanResult *scanResult, issueRange snyk.Range, learnService learn.Service, ep error_reporting.ErrorReporter, fileContent []byte) snyk.Issue { +func toIssue(affectedFilePath string, issue ossIssue, scanResult *scanResult, issueDepNode *ast.Node, learnService learn.Service, ep error_reporting.ErrorReporter, fileContent []byte) snyk.Issue { // this needs to be first so that the lesson from Snyk Learn is added - codeActions := issue.AddCodeActions(learnService, ep, affectedFilePath, issueRange, fileContent) + codeActions := issue.AddCodeActions(learnService, ep, affectedFilePath, issueDepNode, fileContent) var codelensCommands []types.CommandData for _, codeAction := range codeActions { @@ -53,7 +54,7 @@ func toIssue(affectedFilePath string, issue ossIssue, scanResult *scanResult, is Arguments: []any{ codeAction.Uuid, affectedFilePath, - issueRange, + getRangeFromNode(issueDepNode), }, GroupingKey: codeAction.GroupingKey, GroupingType: codeAction.GroupingType, @@ -88,11 +89,12 @@ func toIssue(affectedFilePath string, issue ossIssue, scanResult *scanResult, is if len(message) > maxLength { message = message[:maxLength] + "... (Snyk)" } + d := snyk.Issue{ ID: issue.Id, Message: message, FormattedMessage: issue.GetExtendedMessage(issue), - Range: issueRange, + Range: getRangeFromNode(issueDepNode), Severity: issue.ToIssueSeverity(), AffectedFilePath: affectedFilePath, FileContent: fileContent, @@ -113,7 +115,18 @@ func toIssue(affectedFilePath string, issue ossIssue, scanResult *scanResult, is return d } -func convertScanResultToIssues(res *scanResult, targetFilePath string, fileContent []byte, ls learn.Service, ep error_reporting.ErrorReporter, packageIssueCache map[string][]snyk.Issue) []snyk.Issue { +func getRangeFromNode(issueDepNode *ast.Node) snyk.Range { + if issueDepNode == nil { + return snyk.Range{} + } + r := snyk.Range{ + Start: snyk.Position{Line: issueDepNode.Line, Character: issueDepNode.StartChar}, + End: snyk.Position{Line: issueDepNode.Line, Character: issueDepNode.EndChar}, + } + return r +} + +func convertScanResultToIssues(c *config.Config, res *scanResult, targetFilePath string, fileContent []byte, ls learn.Service, ep error_reporting.ErrorReporter, packageIssueCache map[string][]snyk.Issue) []snyk.Issue { var issues []snyk.Issue duplicateCheckMap := map[string]bool{} @@ -124,8 +137,8 @@ func convertScanResultToIssues(res *scanResult, targetFilePath string, fileConte if duplicateCheckMap[duplicateKey] { continue } - issueRange := findRange(issue, targetFilePath, fileContent) - snykIssue := toIssue(targetFilePath, issue, res, issueRange, ls, ep, fileContent) + node := getDependencyNode(c, targetFilePath, issue, fileContent) + snykIssue := toIssue(targetFilePath, issue, res, node, ls, ep, fileContent) packageIssueCache[packageKey] = append(packageIssueCache[packageKey], snykIssue) issues = append(issues, snykIssue) duplicateCheckMap[duplicateKey] = true diff --git a/infrastructure/oss/maven_range_finder.go b/infrastructure/oss/maven_range_finder.go index e94e4d056..0b99273e7 100644 --- a/infrastructure/oss/maven_range_finder.go +++ b/infrastructure/oss/maven_range_finder.go @@ -18,8 +18,8 @@ package oss import ( "github.com/snyk/snyk-ls/application/config" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/ast/maven" - "github.com/snyk/snyk-ls/domain/snyk" ) type mavenRangeFinder struct { @@ -28,17 +28,14 @@ type mavenRangeFinder struct { c *config.Config } -func (m *mavenRangeFinder) find(issue ossIssue) snyk.Range { - searchPackage, _ := introducingPackageAndVersion(issue) +func (m *mavenRangeFinder) find(introducingPackageName string, introducingVersion string) (*ast.Node, *ast.Tree) { parser := maven.New(m.c) tree := parser.Parse(string(m.fileContent), m.path) for _, depNode := range tree.Root.Children { - if searchPackage == depNode.Name { - return snyk.Range{ - Start: snyk.Position{Line: depNode.Line, Character: depNode.StartChar}, - End: snyk.Position{Line: depNode.Line, Character: depNode.EndChar}, - } + if introducingPackageName == depNode.Name { + // mark, where the dep is mentioned in the file, regardless of parent pom/bom + return depNode, tree } } - return snyk.Range{} + return nil, tree } diff --git a/infrastructure/oss/maven_range_finder_test.go b/infrastructure/oss/maven_range_finder_test.go index 3a86c9a06..40b04ee9f 100644 --- a/infrastructure/oss/maven_range_finder_test.go +++ b/infrastructure/oss/maven_range_finder_test.go @@ -24,7 +24,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/snyk/snyk-ls/application/config" - "github.com/snyk/snyk-ls/domain/snyk" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/internal/testutil" ) @@ -53,17 +53,53 @@ func TestMavenRangeFinder_Find(t *testing.T) { c: c, } - expectedRange := snyk.Range{ - Start: snyk.Position{ - Line: 54, - Character: 15, - }, - End: snyk.Position{ - Line: 54, - Character: 21, - }, + expected := ast.Node{ + Line: 54, + StartChar: 15, + EndChar: 21, } - actualRange := finder.find(issue) - assert.Equal(t, expectedRange, actualRange) + p, v := introducingPackageAndVersion(issue) + + actual, _ := finder.find(p, v) + assert.Equal(t, expected.Line, actual.Line) + assert.Equal(t, expected.StartChar, actual.StartChar) + assert.Equal(t, expected.EndChar, actual.EndChar) +} + +func TestMavenRangeFinder_FindInPomHierarchy(t *testing.T) { + c := testutil.UnitTest(t) + var issue = ossIssue{ + Id: "testIssue", + Name: "SNYK-TEST-ISSUE-1", + Title: "THOU SHALL NOT PASS", + Severity: "1", + LineNumber: 0, + Description: "Getting into Moria is an issue!", + References: nil, + Version: "", + PackageManager: "maven", + From: []string{"goof@1.0.1", "commons-fileupload:commons-fileupload@1.2.1"}, + } + var testPath, _ = filepath.Abs("testdata/maven-goof/sub/subsub/pom.xml") + var testContent, _ = os.ReadFile(testPath) + + finder := mavenRangeFinder{ + path: testPath, + fileContent: testContent, + c: c, + } + + expected := ast.Node{ + Line: 34, + StartChar: 18, + EndChar: 36, + } + + p, v := introducingPackageAndVersion(issue) + + actual, _ := finder.find(p, v) + assert.Equal(t, expected.Line, actual.Line) + assert.Equal(t, expected.StartChar, actual.StartChar) + assert.Equal(t, expected.EndChar, actual.EndChar) } diff --git a/infrastructure/oss/npm_range_finder.go b/infrastructure/oss/npm_range_finder.go index 37d653c9e..bc776a5f0 100644 --- a/infrastructure/oss/npm_range_finder.go +++ b/infrastructure/oss/npm_range_finder.go @@ -20,6 +20,7 @@ import ( "fmt" "strings" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/domain/snyk" ) @@ -29,33 +30,26 @@ type NpmRangeFinder struct { myRange snyk.Range } -func (n *NpmRangeFinder) find(issue ossIssue) snyk.Range { - searchPackage, _ := introducingPackageAndVersion(issue) +func (n *NpmRangeFinder) find(introducingPackageName string, introducingVersion string) (*ast.Node, *ast.Tree) { var lines = strings.Split(strings.ReplaceAll(string(n.fileContent), "\r\n", "\n"), "\n") - var start snyk.Position - var end snyk.Position + node := ast.Node{} for i := 0; i < len(lines); i++ { line := lines[i] elems := strings.Split(line, ":") if len(elems) > 1 { jsonKey := strings.Trim(strings.Trim(elems[0], " "), "\"") - if jsonKey == searchPackage { - start.Line = i - start.Character = strings.Index(line, searchPackage) - 1 - end.Line = i - end.Character = len(strings.ReplaceAll(line, ",", "")) + if jsonKey == introducingPackageName { + node.Line = i + node.StartChar = strings.Index(line, introducingPackageName) - 1 + node.EndChar = len(strings.ReplaceAll(line, ",", "")) break } } } - n.myRange = snyk.Range{ - Start: start, - End: end, - } - return n.myRange + return &node, nil } func introducingPackageAndVersion(issue ossIssue) (string, string) { diff --git a/infrastructure/oss/npm_range_finder_test.go b/infrastructure/oss/npm_range_finder_test.go index 46cdae5b3..3952a6dd4 100644 --- a/infrastructure/oss/npm_range_finder_test.go +++ b/infrastructure/oss/npm_range_finder_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/snyk/snyk-ls/application/config" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/domain/snyk" ) @@ -50,19 +51,24 @@ func TestNpmRangeFinder_Find(t *testing.T) { fileContent: testContent, myRange: snyk.Range{}, } - expectedRange := snyk.Range{ - Start: snyk.Position{ - Line: 17, - Character: 4, - }, - End: snyk.Position{ - Line: 17, - Character: 22, - }, + + expected := ast.Node{ + Line: 17, + StartChar: 4, + EndChar: 22, } - actualRange := npmRangeFinder.find(issue) - assert.Equal(t, expectedRange, actualRange) + executeFinding(t, issue, npmRangeFinder, expected) +} + +func executeFinding(t *testing.T, issue ossIssue, npmRangeFinder NpmRangeFinder, expected ast.Node) { + t.Helper() + p, v := introducingPackageAndVersion(issue) + + actual, _ := npmRangeFinder.find(p, v) + assert.Equal(t, expected.Line, actual.Line) + assert.Equal(t, expected.StartChar, actual.StartChar) + assert.Equal(t, expected.EndChar, actual.EndChar) } func TestNpmRangeFinder_Find_Scoped_Packages(t *testing.T) { @@ -88,17 +94,12 @@ func TestNpmRangeFinder_Find_Scoped_Packages(t *testing.T) { fileContent: testContent, myRange: snyk.Range{}, } - expectedRange := snyk.Range{ - Start: snyk.Position{ - Line: 18, - Character: 4, - }, - End: snyk.Position{ - Line: 18, - Character: 27, - }, + + expected := ast.Node{ + Line: 18, + StartChar: 4, + EndChar: 27, } - actualRange := npmRangeFinder.find(issue) - assert.Equal(t, expectedRange, actualRange) + executeFinding(t, issue, npmRangeFinder, expected) } diff --git a/infrastructure/oss/oss_test.go b/infrastructure/oss/oss_test.go index a13bcf53d..91eb83ced 100644 --- a/infrastructure/oss/oss_test.go +++ b/infrastructure/oss/oss_test.go @@ -30,6 +30,7 @@ import ( "github.com/stretchr/testify/require" "github.com/snyk/snyk-ls/application/config" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/domain/snyk" "github.com/snyk/snyk-ls/infrastructure/cli" "github.com/snyk/snyk-ls/infrastructure/learn" @@ -67,11 +68,13 @@ func Test_determineTargetFile(t *testing.T) { } func Test_FindRange(t *testing.T) { + c := testutil.UnitTest(t) issue := mavenTestIssue() const content = "0\n1\n2\n implementation 'a:test:4.17.4'" var p = "build.gradle" - foundRange := findRange(issue, p, []byte(content)) + node := getDependencyNode(c, p, issue, []byte(content)) + foundRange := getRangeFromNode(node) assert.Equal(t, 3, foundRange.Start.Line) assert.Equal(t, 20, foundRange.Start.Character) @@ -103,7 +106,7 @@ func Test_toIssue_LearnParameterConversion(t *testing.T) { learnService: getLearnMock(t), } - issue := toIssue("testPath", sampleOssIssue, &scanResult{}, snyk.Range{Start: snyk.Position{Line: 1}}, scanner.learnService, scanner.errorReporter, nil) + issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter, nil) assert.Equal(t, sampleOssIssue.Id, issue.ID) assert.Equal(t, sampleOssIssue.Identifiers.CWE, issue.CWEs) @@ -112,8 +115,8 @@ func Test_toIssue_LearnParameterConversion(t *testing.T) { assert.Equal(t, "url", (issue.AdditionalData).(snyk.OssIssueData).Lesson) } -func nonEmptyRange() snyk.Range { - return snyk.Range{Start: snyk.Position{Line: 1}} +func nonEmptyNode() *ast.Node { + return &ast.Node{Line: 1} } func Test_toIssue_CodeActions(t *testing.T) { @@ -144,7 +147,7 @@ func Test_toIssue_CodeActions(t *testing.T) { sampleOssIssue.PackageManager = test.packageManager sampleOssIssue.UpgradePath = []any{"false", test.packageName} - issue := toIssue("testPath", sampleOssIssue, &scanResult{}, snyk.Range{Start: snyk.Position{Line: 1}}, scanner.learnService, scanner.errorReporter, nil) + issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter, nil) assert.Equal(t, sampleOssIssue.Id, issue.ID) assert.Equal(t, flashy+test.expectedUpgrade, issue.CodeActions[0].Title) @@ -170,7 +173,7 @@ func Test_toIssue_CodeActions_WithoutFix(t *testing.T) { } sampleOssIssue.UpgradePath = []any{"*"} - issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyRange(), scanner.learnService, scanner.errorReporter, nil) + issue := toIssue("testPath", sampleOssIssue, &scanResult{}, nonEmptyNode(), scanner.learnService, scanner.errorReporter, nil) assert.Equal(t, sampleOssIssue.Id, issue.ID) assert.Equal(t, 2, len(issue.CodeActions)) diff --git a/infrastructure/oss/parser/parser.go b/infrastructure/oss/parser/parser.go index e8c4cb827..d9f8310a7 100644 --- a/infrastructure/oss/parser/parser.go +++ b/infrastructure/oss/parser/parser.go @@ -20,10 +20,12 @@ import ( "path/filepath" "github.com/snyk/snyk-ls/application/config" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/domain/snyk" ) type Dependency struct { + Tree *ast.Tree GroupID string ArtifactID string Version string diff --git a/infrastructure/oss/range_finder.go b/infrastructure/oss/range_finder.go index b746946bc..c91f4335d 100644 --- a/infrastructure/oss/range_finder.go +++ b/infrastructure/oss/range_finder.go @@ -21,25 +21,27 @@ import ( "strings" "github.com/snyk/snyk-ls/application/config" + "github.com/snyk/snyk-ls/ast" "github.com/snyk/snyk-ls/domain/snyk" ) type RangeFinder interface { - find(issue ossIssue) snyk.Range + find(introducingPackageName string, introducingVersion string) (*ast.Node, *ast.Tree) } + type DefaultFinder struct { path string fileContent []byte c *config.Config } -func findRange(issue ossIssue, path string, fileContent []byte) snyk.Range { - var foundRange snyk.Range +// getDependencyNode will return the dependency node with range information +// in case of maven, the node will also contain tree links information for the whole dep tree +func getDependencyNode(c *config.Config, path string, issue ossIssue, fileContent []byte) *ast.Node { var finder RangeFinder - c := config.CurrentConfig() if len(fileContent) == 0 { - return snyk.Range{Start: snyk.Position{}, End: snyk.Position{}} + return nil } switch issue.PackageManager { @@ -59,33 +61,48 @@ func findRange(issue ossIssue, path string, fileContent []byte) snyk.Range { finder = &DefaultFinder{path: path, fileContent: fileContent, c: c} } - foundRange = finder.find(issue) - return foundRange + introducingPackageName, introducingVersion := introducingPackageAndVersion(issue) + + currentDep, parsedTree := finder.find(introducingPackageName, introducingVersion) + + // if an intermediate manifest file does not have a dependency section + // we go recurse to the parent of it + if currentDep == nil && parsedTree != nil && parsedTree.ParentTree != nil { + tree := parsedTree.ParentTree + currentDep = getDependencyNode(c, tree.Document, issue, []byte(tree.Root.Value)) + } + + // recurse until a dependency with version was found + if currentDep.Value == "" && currentDep.Tree != nil && currentDep.Tree.ParentTree != nil { + tree := currentDep.Tree.ParentTree + currentDep.LinkedParentDependencyNode = getDependencyNode(c, tree.Document, issue, []byte(tree.Root.Value)) + } + + return currentDep } -func (f *DefaultFinder) find(issue ossIssue) snyk.Range { - searchPackage, version := introducingPackageAndVersion(issue) +func (f *DefaultFinder) find(introducingPackageName string, introducingVersion string) (*ast.Node, *ast.Tree) { lines := strings.Split(strings.ReplaceAll(string(f.fileContent), "\r", ""), "\n") for i, line := range lines { if isComment(line) { continue } - if strings.Contains(line, searchPackage) { - endChar := len(strings.TrimRight(strings.TrimRight(strings.TrimRight(line, " "), "\""), "'")) + if strings.Contains(line, introducingPackageName) { + // length of line is ignoring some trailing characters + endChar := len(strings.TrimRight(line, " \"',)")) r := snyk.Range{ - Start: snyk.Position{Line: i, Character: strings.Index(line, searchPackage)}, + Start: snyk.Position{Line: i, Character: strings.Index(line, introducingPackageName)}, End: snyk.Position{Line: i, Character: endChar}, } - f.c.Logger().Debug().Str("package", searchPackage). - Str("version", version). - Str("issueId", issue.Id). + f.c.Logger().Debug().Str("package", introducingPackageName). + Str("version", introducingVersion). Str("path", f.path). Interface("range", r).Msg("found range") - return r + return &ast.Node{Line: r.Start.Line, StartChar: r.Start.Character, EndChar: r.End.Character}, nil } } - return snyk.Range{} + return nil, nil } func isComment(line string) bool { diff --git a/infrastructure/oss/range_finder_test.go b/infrastructure/oss/range_finder_test.go index 073670b88..6cebf6208 100644 --- a/infrastructure/oss/range_finder_test.go +++ b/infrastructure/oss/range_finder_test.go @@ -29,12 +29,13 @@ import ( ) func Test_DefaultFinder_FindRange(t *testing.T) { + c := testutil.UnitTest(t) issue, testPath, testContent := setupDefaultFinderEnvForTesting() expectedRange := getExpectedRangeForDefaultFinderTests() - actualRange := findRange(issue, testPath, testContent) + actualRange := getDependencyNode(c, testPath, issue, testContent) - assert.Equal(t, expectedRange, actualRange) + assert.Equal(t, expectedRange, getRangeFromNode(actualRange)) } func TestDefaultFinder_Find(t *testing.T) { @@ -51,8 +52,10 @@ func TestDefaultFinder_Find(t *testing.T) { expectedRange := getExpectedRangeForDefaultFinderTests() - actualRange := defaultFinder.find(issue) - assert.Equal(t, expectedRange, actualRange) + p, v := introducingPackageAndVersion(issue) + + actualRange, _ := defaultFinder.find(p, v) + assert.Equal(t, expectedRange, getRangeFromNode(actualRange)) } func getExpectedRangeForDefaultFinderTests() snyk.Range { diff --git a/infrastructure/oss/testdata/maven-goof/pom.xml b/infrastructure/oss/testdata/maven-goof/pom.xml new file mode 100644 index 000000000..5478dd1b3 --- /dev/null +++ b/infrastructure/oss/testdata/maven-goof/pom.xml @@ -0,0 +1,38 @@ + + + + 4.0.0 + + com.mycompany.app + my-app + 1.0-SNAPSHOT + pom + + my-app + + http://www.example.com + + + UTF-8 + 1.7 + 1.7 + + + + + + junit + junit + 4.11 + test + + + commons-fileupload + commons-fileupload + 1.2.1 + compile + + + + diff --git a/infrastructure/oss/testdata/maven-goof/sub/pom.xml b/infrastructure/oss/testdata/maven-goof/sub/pom.xml new file mode 100644 index 000000000..e662500b7 --- /dev/null +++ b/infrastructure/oss/testdata/maven-goof/sub/pom.xml @@ -0,0 +1,74 @@ + + + + 4.0.0 + + com.mycompany.app + my-child-app + 1.0-SNAPSHOT + + + my-app + + http://www.example.com + + + com.mycompany.app + my-app + 1.0-SNAPSHOT + + + + + UTF-8 + 1.7 + 1.7 + + + + + + + + maven-clean-plugin + 3.1.0 + + + + maven-resources-plugin + 3.0.2 + + + maven-compiler-plugin + 3.8.0 + + + maven-surefire-plugin + 2.22.1 + + + maven-jar-plugin + 3.0.2 + + + maven-install-plugin + 2.5.2 + + + maven-deploy-plugin + 2.8.2 + + + + maven-site-plugin + 3.7.1 + + + maven-project-info-reports-plugin + 3.0.0 + + + + + diff --git a/infrastructure/oss/testdata/maven-goof/sub/subsub/pom.xml b/infrastructure/oss/testdata/maven-goof/sub/subsub/pom.xml new file mode 100644 index 000000000..be71849a3 --- /dev/null +++ b/infrastructure/oss/testdata/maven-goof/sub/subsub/pom.xml @@ -0,0 +1,84 @@ + + + + 4.0.0 + + com.mycompany.app + my-child-child-app + 1.0-SNAPSHOT + + + my-app + + http://www.example.com + + + com.mycompany.app + my-child-app + 1.0-SNAPSHOT + + + + UTF-8 + 1.7 + 1.7 + + + + + junit + junit + + + commons-fileupload + commons-fileupload + + + + + + + + + maven-clean-plugin + 3.1.0 + + + + maven-resources-plugin + 3.0.2 + + + maven-compiler-plugin + 3.8.0 + + + maven-surefire-plugin + 2.22.1 + + + maven-jar-plugin + 3.0.2 + + + maven-install-plugin + 2.5.2 + + + maven-deploy-plugin + 2.8.2 + + + + maven-site-plugin + 3.7.1 + + + maven-project-info-reports-plugin + 3.0.0 + + + + +