diff --git a/commands/audit/sca/pnpm/pnpm_test.go b/commands/audit/sca/pnpm/pnpm_test.go index a9bfba186..3b0625f99 100644 --- a/commands/audit/sca/pnpm/pnpm_test.go +++ b/commands/audit/sca/pnpm/pnpm_test.go @@ -99,26 +99,27 @@ func TestBuildDependencyTree(t *testing.T) { expectedUniqueDeps: []string{ "npm://jfrog-cli-tests:v1.0.0", "npm://xml:1.0.1", - "npm://json:9.0.6", + "npm://json:9.0.3", }, expectedTree: &xrayUtils.GraphNode{ Id: "npm://jfrog-cli-tests:v1.0.0", Nodes: []*xrayUtils.GraphNode{ {Id: "npm://xml:1.0.1"}, - {Id: "npm://json:9.0.6"}, + {Id: "npm://json:9.0.3"}, }, }, }, { name: "Prod", depType: "prodOnly", + expectedUniqueDeps: []string{ "npm://jfrog-cli-tests:v1.0.0", - "npm://xml:1.0.1", + "npm://json:9.0.3", }, expectedTree: &xrayUtils.GraphNode{ Id: "npm://jfrog-cli-tests:v1.0.0", - Nodes: []*xrayUtils.GraphNode{{Id: "npm://xml:1.0.1"}}, + Nodes: []*xrayUtils.GraphNode{{Id: "npm://json:9.0.3"}}, }, }, { @@ -126,11 +127,11 @@ func TestBuildDependencyTree(t *testing.T) { depType: "devOnly", expectedUniqueDeps: []string{ "npm://jfrog-cli-tests:v1.0.0", - "npm://json:9.0.6", + "npm://xml:1.0.1", }, expectedTree: &xrayUtils.GraphNode{ Id: "npm://jfrog-cli-tests:v1.0.0", - Nodes: []*xrayUtils.GraphNode{{Id: "npm://json:9.0.6"}}, + Nodes: []*xrayUtils.GraphNode{{Id: "npm://xml:1.0.1"}}, }, }, } diff --git a/sca/npm/npmhandler.go b/sca/npm/npmhandler.go new file mode 100644 index 000000000..c918770de --- /dev/null +++ b/sca/npm/npmhandler.go @@ -0,0 +1,99 @@ +package npm + +import ( + "fmt" + "regexp" + "strings" + + "github.com/jfrog/jfrog-cli-security/utils/formats/sarifutils" + "github.com/jfrog/jfrog-client-go/utils/io/fileutils" + "github.com/owenrumney/go-sarif/v2/sarif" +) + +const ( + PackageJson = "package.json" +) + +type NpmHandler struct{} + +func (nh *NpmHandler) GetTechDependencyLocations(directDependencyName, directDependencyVersion string, filesToSearch ...string) (locations []*sarif.Location, err error) { + for _, file := range getFilesToSearch(filesToSearch...) { + fileLocations, err := getDependencyLocations(file, directDependencyName, directDependencyVersion) + if err != nil { + return nil, err + } + locations = append(locations, fileLocations...) + } + return +} + +// getFilesToSearch returns the npm related files to search for the dependency +// If no files are provided, the default is to search for package.json at the current directory +func getFilesToSearch(filesToSearch ...string) (out []string) { + if len(filesToSearch) == 0 { + if fileutils.IsPathExists(PackageJson, false) { + out = append(out, PackageJson) + } + return + } + for _, file := range filesToSearch { + potentialPath := strings.TrimSuffix(file, "/") + if strings.HasSuffix(potentialPath, PackageJson) { + out = append(out, potentialPath) + } + } + return out +} + +// getDependencyLocations returns the locations of the dependency in the file +func getDependencyLocations(file, directDependencyName, dependencyVersion string) (locations []*sarif.Location, err error) { + content, err := fileutils.ReadFile(file) + if err != nil { + return nil, fmt.Errorf("failed to read file '%s': %v", file, err) + } + + // Prepare regular expression to match all possible ways to specify a version. + pattern := fmt.Sprintf(`"%s"\s*:\s*"([~^]?\d+(?:\.\d+)?(?:\.\d+)?)"`, regexp.QuoteMeta(directDependencyName)) + + // Compile the regex. + re := regexp.MustCompile(pattern) + + // Split the contents into lines for processing. + lines := strings.Split(string(content), "\n") + + for lineNumber, line := range lines { + // Find all match locations in the line. + matches := re.FindStringSubmatch(line) + if matches != nil { + // Extract detected version from match + detectedVersion := matches[1] + + // Normalize the given dependency version by allowing optional ~ or ^ prefix + if dependencyVersion != "" { + allowedPrefixes := []string{"", "~", "^"} + matchFound := false + for _, prefix := range allowedPrefixes { + if strings.HasPrefix(prefix+dependencyVersion, detectedVersion) { + matchFound = true + break + } + } + if !matchFound { + // Skip if the provided version does not match the detected version + continue + } + } + + // Get the matched snippet + matchIndex := re.FindStringIndex(line) + matchedSnippet := line[matchIndex[0]:matchIndex[1]] + + // Rows and Cols values are 1-based. (min value is 1) + row := lineNumber + 1 + startCol := matchIndex[0] + 1 + locations = append(locations, sarifutils.CreateLocation(file, row, startCol, row, startCol+len(matchedSnippet), matchedSnippet)) + } + } + + return locations, nil +} diff --git a/sca/npm/npmhandler_test.go b/sca/npm/npmhandler_test.go new file mode 100644 index 000000000..ce64ba59b --- /dev/null +++ b/sca/npm/npmhandler_test.go @@ -0,0 +1,78 @@ +package npm + +import ( + "path/filepath" + "testing" + + "github.com/owenrumney/go-sarif/v2/sarif" + "github.com/stretchr/testify/assert" + + securityTestUtils "github.com/jfrog/jfrog-cli-security/tests/utils" + "github.com/jfrog/jfrog-cli-security/utils/formats/sarifutils" +) + +var ( + testDataDir = filepath.Join("..", "..", "tests", "testdata", "projects", "package-managers", "npm") +) + +func TestNpmGetTechDependencyLocations(t *testing.T) { + cleanUp := securityTestUtils.ChangeWDWithCallback(t, filepath.Join(testDataDir, "npm")) + defer cleanUp() + + testCases := []struct { + name string + directDependencyName string + directDependencyVersion string + filesToSearch []string + expectedLocations []*sarif.Location + expectedError error + }{ + { + name: "dependency not found", + directDependencyName: "json", + filesToSearch: []string{filepath.Join("..", "npm-scripts", "package.json")}, + }, + { + name: "dependency all versions", + directDependencyName: "json", + filesToSearch: []string{ + "package.json", + filepath.Join("..", "npm-big-tree", "package.json"), + filepath.Join("..", "npm-no-lock", "package.json"), + }, + expectedLocations: []*sarif.Location{ + sarifutils.CreateLocation("package.json", 15, 5, 15, 20, "\"json\": \"9.0.6\""), + sarifutils.CreateLocation(filepath.Join("..", "npm-no-lock", "package.json"), 12, 5, 12, 20, "\"json\": \"9.0.3\""), + }, + }, + { + name: "dependency specific version", + directDependencyName: "json", + directDependencyVersion: "9.0.6", + filesToSearch: []string{ + "package.json", + filepath.Join("..", "npm-scripts", "package.json"), + filepath.Join("..", "npm-no-lock", "package.json"), + }, + expectedLocations: []*sarif.Location{ + sarifutils.CreateLocation("package.json", 15, 5, 15, 20, "\"json\": \"9.0.6\""), + }, + }, + { + name: "search at cwd (no files to search)", + directDependencyName: "xml", + expectedLocations: []*sarif.Location{ + sarifutils.CreateLocation("package.json", 12, 5, 12, 19, "\"xml\": \"1.0.1\""), + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + nh := NpmHandler{} + locations, err := nh.GetTechDependencyLocations(tc.directDependencyName, tc.directDependencyVersion, tc.filesToSearch...) + assert.ElementsMatch(t, tc.expectedLocations, locations) + assert.Equal(t, tc.expectedError, err) + }) + } +} diff --git a/sca/sca.go b/sca/sca.go new file mode 100644 index 000000000..aa4359e90 --- /dev/null +++ b/sca/sca.go @@ -0,0 +1,16 @@ +package sca + +import ( + "github.com/owenrumney/go-sarif/v2/sarif" + + "github.com/jfrog/jfrog-cli-security/sca/npm" + "github.com/jfrog/jfrog-cli-security/utils/techutils" +) + +func GetTechDependencyLocations(tech techutils.Technology, directDependencyName, directDependencyVersion string, filesToSearch ...string) ([]*sarif.Location, error) { + if tech == techutils.Npm { + nh := npm.NpmHandler{} + return nh.GetTechDependencyLocations(directDependencyName, directDependencyVersion, filesToSearch...) + } + return nil, nil +} diff --git a/tests/testdata/projects/package-managers/npm/npm-no-lock/package.json b/tests/testdata/projects/package-managers/npm/npm-no-lock/package.json index afd6275ac..1125fd2f7 100755 --- a/tests/testdata/projects/package-managers/npm/npm-no-lock/package.json +++ b/tests/testdata/projects/package-managers/npm/npm-no-lock/package.json @@ -9,9 +9,9 @@ "author": "", "license": "ISC", "dependencies": { - "xml": "1.0.1" + "json": "9.0.3" }, "devDependencies": { - "json": "9.0.6" + "xml": "1.0.1" } } \ No newline at end of file diff --git a/utils/results/common.go b/utils/results/common.go index 784b519ee..03b2624df 100644 --- a/utils/results/common.go +++ b/utils/results/common.go @@ -12,6 +12,7 @@ import ( "github.com/jfrog/gofrog/datastructures" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" + "github.com/jfrog/jfrog-cli-security/sca" "github.com/jfrog/jfrog-cli-security/utils" "github.com/jfrog/jfrog-cli-security/utils/formats" "github.com/jfrog/jfrog-cli-security/utils/formats/sarifutils" @@ -98,7 +99,7 @@ func ApplyHandlerToScaVulnerabilities(target ScanTarget, vulnerabilities []servi return nil } for _, vulnerability := range vulnerabilities { - impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes, fixedVersions, directComponents, impactPaths, err := SplitComponents(target.Target, vulnerability.Components) + impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes, fixedVersions, directComponents, impactPaths, err := SplitComponents(target, vulnerability.Components) if err != nil { return err } @@ -131,7 +132,7 @@ func ApplyHandlerToScaViolations(target ScanTarget, violations []services.Violat watchesSet.Add(violation.WatchName) failBuild = failBuild || violation.FailBuild // Prepare violation information - impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes, fixedVersions, directComponents, impactPaths, e := SplitComponents(target.Target, violation.Components) + impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes, fixedVersions, directComponents, impactPaths, e := SplitComponents(target, violation.Components) if e != nil { err = errors.Join(err, e) continue @@ -212,7 +213,7 @@ func ApplyHandlerToLicenses(target ScanTarget, licenses []services.License, hand return nil } for _, license := range licenses { - impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes, _, directComponents, impactPaths, err := SplitComponents(target.Target, license.Components) + impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes, _, directComponents, impactPaths, err := SplitComponents(target, license.Components) if err != nil { return err } @@ -227,7 +228,7 @@ func ApplyHandlerToLicenses(target ScanTarget, licenses []services.License, hand return nil } -func SplitComponents(target string, impactedPackages map[string]services.Component) (impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes []string, fixedVersions [][]string, directComponents [][]formats.ComponentRow, impactPaths [][][]formats.ComponentRow, err error) { +func SplitComponents(target ScanTarget, impactedPackages map[string]services.Component) (impactedPackagesNames, impactedPackagesVersions, impactedPackagesTypes []string, fixedVersions [][]string, directComponents [][]formats.ComponentRow, impactPaths [][][]formats.ComponentRow, err error) { if len(impactedPackages) == 0 { err = errorutils.CheckErrorf("failed while parsing the response from Xray: violation doesn't have any components") return @@ -246,7 +247,7 @@ func SplitComponents(target string, impactedPackages map[string]services.Compone } // Gets a slice of the direct dependencies or packages of the scanned component, that depends on the vulnerable package, and converts the impact paths. -func getDirectComponentsAndImpactPaths(target string, impactPaths [][]services.ImpactPathNode) (components []formats.ComponentRow, impactPathsRows [][]formats.ComponentRow) { +func getDirectComponentsAndImpactPaths(target ScanTarget, impactPaths [][]services.ImpactPathNode) (components []formats.ComponentRow, impactPathsRows [][]formats.ComponentRow) { componentsMap := make(map[string]formats.ComponentRow) // The first node in the impact path is the scanned component itself. The second one is the direct dependency. @@ -259,7 +260,7 @@ func getDirectComponentsAndImpactPaths(target string, impactPaths [][]services.I componentId := impactPath[impactPathIndex].ComponentId if _, exist := componentsMap[componentId]; !exist { compName, compVersion, _ := techutils.SplitComponentId(componentId) - componentsMap[componentId] = formats.ComponentRow{Name: compName, Version: compVersion, Location: getComponentLocation(impactPath[impactPathIndex].FullPath, target)} + componentsMap[componentId] = formats.ComponentRow{Name: compName, Version: compVersion, Location: getComponentLocation(target.Technology, compName, compVersion, impactPath[impactPathIndex].FullPath, target.Target)} } // Convert the impact path @@ -269,7 +270,7 @@ func getDirectComponentsAndImpactPaths(target string, impactPaths [][]services.I compImpactPathRows = append(compImpactPathRows, formats.ComponentRow{ Name: nodeCompName, Version: nodeCompVersion, - Location: getComponentLocation(pathNode.FullPath), + Location: getComponentLocation(target.Technology, nodeCompName, nodeCompVersion, pathNode.FullPath), }) } impactPathsRows = append(impactPathsRows, compImpactPathRows) @@ -281,7 +282,22 @@ func getDirectComponentsAndImpactPaths(target string, impactPaths [][]services.I return } -func getComponentLocation(pathsByPriority ...string) *formats.Location { +func getComponentLocation(tech techutils.Technology, compName, compVersion string, pathsByPriority ...string) *formats.Location { + // Search location at descriptors + locations, err := sca.GetTechDependencyLocations(tech, compName, compVersion, pathsByPriority...) + if err == nil && len(locations) > 0 { + // Use only one of the detected locations (not supporting multiple locations for now) + location := locations[0] + return &formats.Location{ + File: sarifutils.GetLocationFileName(location), + StartLine: sarifutils.GetLocationStartLine(location), + StartColumn: sarifutils.GetLocationStartColumn(location), + EndLine: sarifutils.GetLocationEndLine(location), + EndColumn: sarifutils.GetLocationEndColumn(location), + Snippet: sarifutils.GetLocationSnippetText(location), + } + } + // Fallback to given paths without region for _, path := range pathsByPriority { if path != "" { return &formats.Location{File: path} diff --git a/utils/results/common_test.go b/utils/results/common_test.go index e4492560b..6a0d8bdca 100644 --- a/utils/results/common_test.go +++ b/utils/results/common_test.go @@ -625,7 +625,7 @@ func TestShouldDisqualifyEvidence(t *testing.T) { func TestGetDirectComponents(t *testing.T) { tests := []struct { name string - target string + target ScanTarget impactPaths [][]services.ImpactPathNode expectedDirectComponentRows []formats.ComponentRow expectedConvImpactPaths [][]formats.ComponentRow @@ -638,14 +638,14 @@ func TestGetDirectComponents(t *testing.T) { }, { name: "one direct component with target", - target: filepath.Join("root", "dir", "file"), + target: ScanTarget{Target: filepath.Join("root", "dir", "file")}, impactPaths: [][]services.ImpactPathNode{{services.ImpactPathNode{ComponentId: "gav://jfrog:pack1:1.2.3"}, services.ImpactPathNode{ComponentId: "gav://jfrog:pack2:1.2.3"}}}, expectedDirectComponentRows: []formats.ComponentRow{{Name: "jfrog:pack2", Version: "1.2.3", Location: &formats.Location{File: filepath.Join("root", "dir", "file")}}}, expectedConvImpactPaths: [][]formats.ComponentRow{{{Name: "jfrog:pack1", Version: "1.2.3"}, {Name: "jfrog:pack2", Version: "1.2.3"}}}, }, { name: "multiple direct components", - target: filepath.Join("root", "dir", "file"), + target: ScanTarget{Target: filepath.Join("root", "dir", "file")}, impactPaths: [][]services.ImpactPathNode{{services.ImpactPathNode{ComponentId: "gav://jfrog:pack1:1.2.3"}, services.ImpactPathNode{ComponentId: "gav://jfrog:pack21:1.2.3"}, services.ImpactPathNode{ComponentId: "gav://jfrog:pack3:1.2.3"}}, {services.ImpactPathNode{ComponentId: "gav://jfrog:pack1:1.2.3"}, services.ImpactPathNode{ComponentId: "gav://jfrog:pack22:1.2.3"}, services.ImpactPathNode{ComponentId: "gav://jfrog:pack3:1.2.3"}}}, expectedDirectComponentRows: []formats.ComponentRow{ {Name: "jfrog:pack21", Version: "1.2.3", Location: &formats.Location{File: filepath.Join("root", "dir", "file")}}, diff --git a/utils/results/conversion/sarifparser/sarifparser.go b/utils/results/conversion/sarifparser/sarifparser.go index 95c24dfd9..1346ae186 100644 --- a/utils/results/conversion/sarifparser/sarifparser.go +++ b/utils/results/conversion/sarifparser/sarifparser.go @@ -499,14 +499,20 @@ func getScaIssueSarifRule(impactPaths [][]formats.ComponentRow, ruleId, ruleDesc func getComponentSarifLocation(cmtType utils.CommandType, component formats.ComponentRow) *sarif.Location { filePath := "" + physicalLocation := sarif.NewPhysicalLocation() if component.Location != nil { filePath = component.Location.File + if component.Location.StartLine != 0 || component.Location.EndLine != 0 || component.Location.StartColumn != 0 || component.Location.EndColumn != 0 { + // Region is not empty, add it to the location + physicalLocation.WithRegion(sarif.NewRegion().WithStartLine(component.Location.StartLine).WithStartColumn(component.Location.StartColumn).WithEndLine(component.Location.EndLine).WithEndColumn(component.Location.EndColumn).WithSnippet(sarif.NewArtifactContent().WithText(component.Location.Snippet))) + } } if strings.TrimSpace(filePath) == "" { // For tech that we don't support fetching the package descriptor related to the component filePath = "Package-Descriptor" } - var logicalLocations []*sarif.LogicalLocation + location := sarif.NewLocation().WithPhysicalLocation(physicalLocation.WithArtifactLocation(sarif.NewSimpleArtifactLocation(fmt.Sprintf("file://%s", filePath)))) + if cmtType == utils.DockerImage { // Docker image - extract layer hash from component name algorithm, layer := getLayerContentFromComponentId(component.Name) @@ -515,11 +521,11 @@ func getComponentSarifLocation(cmtType utils.CommandType, component formats.Comp if algorithm != "" { logicalLocation.Properties = map[string]interface{}{"algorithm": algorithm} } - logicalLocations = append(logicalLocations, logicalLocation) + location.WithLogicalLocations([]*sarif.LogicalLocation{logicalLocation}) } } - return sarif.NewLocation(). - WithPhysicalLocation(sarif.NewPhysicalLocation().WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://" + filePath))).WithLogicalLocations(logicalLocations) + + return location } func getScaIssueMarkdownDescription(directDependencies []formats.ComponentRow, cveScore string, applicableStatus jasutils.ApplicabilityStatus, fixedVersions []string) (string, error) { diff --git a/utils/results/conversion/sarifparser/sarifparser_test.go b/utils/results/conversion/sarifparser/sarifparser_test.go index 19871a5ad..724dc2788 100644 --- a/utils/results/conversion/sarifparser/sarifparser_test.go +++ b/utils/results/conversion/sarifparser/sarifparser_test.go @@ -55,6 +55,17 @@ func TestGetComponentSarifLocation(t *testing.T) { WithArtifactLocation(sarif.NewArtifactLocation().WithUri("file://Package-Descriptor")), ).WithLogicalLocations([]*sarif.LogicalLocation{sarifutils.CreateLogicalLocationWithProperty("3a8bca98bcad879bca98b9acd", "layer", "algorithm", "sha256")}), }, + { + name: "Component with location and region", + component: formats.ComponentRow{ + Name: "example-package", + Version: "1.0.0", + Location: &formats.Location{File: filepath.Join("dir", "file.txt"), StartLine: 1, StartColumn: 2, EndLine: 3, EndColumn: 4, Snippet: "Snippet"}, + }, + expectedOutput: sarif.NewLocation().WithPhysicalLocation(sarif.NewPhysicalLocation(). + WithArtifactLocation(sarif.NewArtifactLocation().WithUri(fmt.Sprintf("file://%s", filepath.Join("dir", "file.txt")))).WithRegion(sarif.NewRegion().WithStartLine(1).WithStartColumn(2).WithEndLine(3).WithEndColumn(4).WithSnippet(sarif.NewArtifactContent().WithText("Snippet"))), + ), + }, } for _, tc := range testCases { diff --git a/utils/techutils/techutils.go b/utils/techutils/techutils.go index 331c23a78..9173d706c 100644 --- a/utils/techutils/techutils.go +++ b/utils/techutils/techutils.go @@ -13,6 +13,8 @@ import ( "golang.org/x/text/cases" "golang.org/x/text/language" + "github.com/owenrumney/go-sarif/v2/sarif" + "github.com/jfrog/gofrog/datastructures" "github.com/jfrog/jfrog-cli-core/v2/common/project" "github.com/jfrog/jfrog-cli-core/v2/utils/coreutils" @@ -95,6 +97,11 @@ var packageTypes = map[string]string{ "alpine": "Alpine", } +type TechnologyHandler interface { + // Get the locations of the direct dependency in the given files. if none were given, search at the cwd + GetTechDependencyLocations(directDependencyName, directDependencyVersion string, filesToSearch ...string) ([]*sarif.Location, error) +} + type TechData struct { // The name of the package type used in this technology. packageType string