Skip to content

Commit

Permalink
Yarn - Refactor the process of building impact graph (#440)
Browse files Browse the repository at this point in the history 
* Depends on Resolved Yarn 1 impact graph issue displaying hoisted paths ide-plugins-common#144
* note to change settings.gradle & build.gradle before merge
* addImpactPathToDependencyNode increment logic was fixed in ScannerBase.java
* walkDepTree new logic using 'Yarn why' command was added using YarnScanner.java
* YarnScanerTests.java was added with new resources in yarn.exampleYarnProject dir
  • Loading branch information
@noyshabtay
noyshabtay authored Nov 28, 2023
1 parent 6e29664 commit afcd52b
Show file tree
Hide file tree
Showing 17 changed files with 2,259 additions and 26 deletions.
2 changes: 1 addition & 1 deletion build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ repositories {
}

def buildInfoVersion = '2.41.6'
def idePluginsCommonVersion = '2.3.1'
def idePluginsCommonVersion = '2.3.2'

dependencies {
implementation group: 'com.fasterxml.jackson.dataformat', name: 'jackson-dataformat-yaml', version: '2.15.2'
Expand Down
25 changes: 12 additions & 13 deletions src/main/java/com/jfrog/ide/idea/scan/ScannerBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@
* Created by romang on 4/26/17.
*/
public abstract class ScannerBase {
public static final int IMPACT_PATHS_LIMIT = 50;

private final ServerConfig serverConfig;
private final ComponentPrefix prefix;
@Getter
Expand Down Expand Up @@ -159,7 +157,7 @@ private void scanAndUpdate(ProgressIndicator indicator) {
// No violations/vulnerabilities or no components to scan or an error was thrown
return;
}
List<FileTreeNode> fileTreeNodes = walkDepTree(results, depTree);
List<FileTreeNode> fileTreeNodes = buildImpactGraph(results, depTree);
addScanResults(fileTreeNodes);

// Contextual Analysis
Expand All @@ -185,9 +183,9 @@ private void scanAndUpdate(ProgressIndicator indicator) {
* @param vulnerableDependencies a map of component IDs and the DependencyNode object matching each of them.
* @param depTree the project's dependency tree to walk through.
*/
private List<FileTreeNode> walkDepTree(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) {
protected List<FileTreeNode> buildImpactGraph(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) throws IOException {
Map<String, DescriptorFileTreeNode> descriptorNodes = new HashMap<>();
visitDepTreeNode(vulnerableDependencies, depTree, Collections.singletonList(depTree.getRootId()), descriptorNodes, new ArrayList<>(), new HashMap<>());
visitDepTreeNode(vulnerableDependencies, depTree, Collections.singletonList(depTree.rootId()), descriptorNodes, new ArrayList<>(), new HashMap<>());
return new CopyOnWriteArrayList<>(descriptorNodes.values());
}

Expand All @@ -207,7 +205,7 @@ private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies
Map<String, DescriptorFileTreeNode> descriptorNodes, List<String> descriptorPaths,
Map<String, Map<String, DependencyNode>> addedDeps) {
String compId = path.get(path.size() - 1);
DepTreeNode compNode = depTree.getNodes().get(compId);
DepTreeNode compNode = depTree.nodes().get(compId);
List<String> innerDescriptorPaths = descriptorPaths;
if (compNode.getDescriptorFilePath() != null) {
innerDescriptorPaths = new ArrayList<>(descriptorPaths);
Expand All @@ -220,7 +218,7 @@ private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies
DepTreeNode parentCompNode = null;
if (path.size() >= 2) {
String parentCompId = path.get(path.size() - 2);
parentCompNode = depTree.getNodes().get(parentCompId);
parentCompNode = depTree.nodes().get(parentCompId);
}
for (String descriptorPath : innerDescriptorPaths) {
boolean indirect = parentCompNode != null && !descriptorPath.equals(parentCompNode.getDescriptorFilePath());
Expand Down Expand Up @@ -255,13 +253,10 @@ private void visitDepTreeNode(Map<String, DependencyNode> vulnerableDependencies
}
}

private void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<String> path) {
if (dependencyNode.getImpactTree() == null) {
dependencyNode.setImpactTree(new ImpactTree(new ImpactTreeNode(path.get(0))));
}
protected void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<String> path) {
dependencyNode.setImpactTree(new ImpactTree(new ImpactTreeNode(path.get(0))));
ImpactTree impactTree = dependencyNode.getImpactTree();
impactTree.incImpactPathsCount();
if (impactTree.getImpactPathsCount() > IMPACT_PATHS_LIMIT) {
if (impactTree.getImpactPathsCount() == ImpactTree.IMPACT_PATHS_LIMIT) {
return;
}
ImpactTreeNode parentImpactTreeNode = impactTree.getRoot();
Expand All @@ -272,6 +267,10 @@ private void addImpactPathToDependencyNode(DependencyNode dependencyNode, List<S
if (currImpactTreeNode == null) {
currImpactTreeNode = new ImpactTreeNode(currPathNode);
parentImpactTreeNode.getChildren().add(currImpactTreeNode);
if (pathNodeIndex == path.size() - 1) {
// If a new leaf was added, thus a new impact path was added (impact paths don't collide after they split)
impactTree.incImpactPathsCount();
}
}
parentImpactTreeNode = currImpactTreeNode;
}
Expand Down
79 changes: 77 additions & 2 deletions src/main/java/com/jfrog/ide/idea/scan/YarnScanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
import com.intellij.psi.PsiManager;
import com.intellij.util.EnvironmentUtil;
import com.jfrog.ide.common.deptree.DepTree;
import com.jfrog.ide.common.nodes.DependencyNode;
import com.jfrog.ide.common.nodes.DescriptorFileTreeNode;
import com.jfrog.ide.common.nodes.FileTreeNode;
import com.jfrog.ide.common.scan.ComponentPrefix;
import com.jfrog.ide.common.scan.ScanLogic;
import com.jfrog.ide.common.yarn.YarnTreeBuilder;
Expand All @@ -15,9 +18,13 @@
import com.jfrog.ide.idea.scan.data.PackageManagerType;
import com.jfrog.ide.idea.ui.ComponentsTree;
import com.jfrog.ide.idea.ui.menus.filtermanager.ConsistentFilterManager;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;

import java.io.IOException;
import java.nio.file.Paths;
import java.util.*;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.ExecutorService;

/**
Expand All @@ -26,6 +33,7 @@
public class YarnScanner extends SingleDescriptorScanner {

private final YarnTreeBuilder yarnTreeBuilder;

/**
* @param project currently opened IntelliJ project. We'll use this project to retrieve project based services
* like {@link ConsistentFilterManager} and {@link ComponentsTree}.
Expand All @@ -36,12 +44,12 @@ public class YarnScanner extends SingleDescriptorScanner {
YarnScanner(Project project, String basePath, ExecutorService executor, ScanLogic scanLogic) {
super(project, basePath, ComponentPrefix.NPM, executor, Paths.get(basePath, "package.json").toString(), scanLogic);
getLog().info("Found yarn project: " + getProjectPath());
yarnTreeBuilder = new YarnTreeBuilder(Paths.get(basePath), descriptorFilePath, EnvironmentUtil.getEnvironmentMap());
yarnTreeBuilder = new YarnTreeBuilder(Paths.get(basePath), descriptorFilePath, EnvironmentUtil.getEnvironmentMap(), getLog());
}

@Override
protected DepTree buildTree() throws IOException {
return yarnTreeBuilder.buildTree(getLog());
return yarnTreeBuilder.buildTree();
}

@Override
Expand All @@ -63,5 +71,72 @@ protected AbstractInspection getInspectionTool() {
protected PackageManagerType getPackageManagerType() {
return PackageManagerType.YARN;
}

/**
* Builds a map of package name to versions out of a set of <package-name>:<version> Strings.
*
* @param packages - A set of packages in the format of 'package-name:version'.
* @return - A map of package name to a set of versions.
*/
Map<String, Set<String>> getPackageNameToVersionsMap(Set<String> packages) {
Map<String, Set<String>> packageNameToVersions = new HashMap<>();
for (String fullNamePackage : CollectionUtils.emptyIfNull(packages)) {
String[] packageSplit = StringUtils.split(fullNamePackage, ":");
if (packageSplit.length != 2) {
this.getLog().error("Illegal package name: " + fullNamePackage + ". Skipping package, the dependency tree may be incomplete.");
continue;
}
String packageName = packageSplit[0];
String packageVersion = packageSplit[1];
packageNameToVersions.putIfAbsent(packageName, new HashSet<>());
packageNameToVersions.get(packageName).add(packageVersion);
}
return packageNameToVersions;
}

private void buildImpactGraphFromPaths(DescriptorFileTreeNode descriptorNode, Map<String, DependencyNode> vulnerableDependencies, Map<String, List<List<String>>> packageVersionsImpactPaths) {
for (Map.Entry<String, List<List<String>>> aPackageVersionImpactPaths : packageVersionsImpactPaths.entrySet()) {
String packageFullName = aPackageVersionImpactPaths.getKey();
List<List<String>> impactPaths = aPackageVersionImpactPaths.getValue();
DependencyNode dependencyNode = vulnerableDependencies.get(packageFullName);

// build the impact graph for each vulnerable dependency out of its impact paths
for (List<String> impactPath : impactPaths) {
this.addImpactPathToDependencyNode(dependencyNode, impactPath);
}

boolean direct = impactPaths.stream().map(List::size).anyMatch(size -> size == 2);

dependencyNode.setIndirect(!direct);
descriptorNode.addDependency(dependencyNode);
}
}

/**
* Builds the impact graph for each given vulnerable dependencies.
* The impact graph is built by running 'yarn why <package>' command, making it different from other package managers.
*
* @param vulnerableDependencies - The vulnerable dependencies to build the impact graph for.
* The key is the package name and version, and the value is the dependency node.
* @param depTree - The whole dependency tree (not just vulnerable dependencies) that was generated earlier.
* @return - The impact graph attached to package.json DescriptorFileTreeNode
*/
@Override
protected List<FileTreeNode> buildImpactGraph(Map<String, DependencyNode> vulnerableDependencies, DepTree depTree) throws IOException {
DescriptorFileTreeNode descriptorNode = new DescriptorFileTreeNode(depTree.getRootNodeDescriptorFilePath());
// Build a map of package name to versions, to avoid running 'yarn why' multiple times for the same package.
Map<String, Set<String>> packageNameToVersions = this.getPackageNameToVersionsMap(vulnerableDependencies.keySet());

for (Map.Entry<String, Set<String>> entry : packageNameToVersions.entrySet()) {
// Find the impact paths for each package for all its vulnerable versions
Map<String, List<List<String>>> packageVersionsImpactPaths = yarnTreeBuilder.findDependencyImpactPaths(depTree.rootId(), entry.getKey(), entry.getValue());
// Build the impact graph for each vulnerable dependency out of its impact paths, set Indirect flag and add it to the descriptor node
buildImpactGraphFromPaths(descriptorNode, vulnerableDependencies, packageVersionsImpactPaths);
}

// Return a list of one element - the descriptor node for package.json
// COW list is used to avoid ConcurrentModificationException in SourceCodeScannerManager
return new CopyOnWriteArrayList<>(Collections.singletonList(descriptorNode));
}
}

3 changes: 1 addition & 2 deletions src/main/java/com/jfrog/ide/idea/ui/webview/WebviewObjectConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.jfrog.ide.common.nodes.*;
import com.jfrog.ide.common.nodes.subentities.*;
import com.jfrog.ide.common.scan.ComponentPrefix;
import com.jfrog.ide.idea.scan.ScannerBase;
import com.jfrog.ide.idea.ui.webview.model.Cve;
import com.jfrog.ide.idea.ui.webview.model.Evidence;
import com.jfrog.ide.idea.ui.webview.model.License;
Expand Down Expand Up @@ -143,7 +142,7 @@ public static DependencyPage convertLicenseToDepPage(LicenseViolationNode licens
}

private static ImpactGraph convertImpactGraph(ImpactTree impactTree) {
return new ImpactGraph(convertImpactGraphNode(impactTree.getRoot()), impactTree.getImpactPathsCount(), ScannerBase.IMPACT_PATHS_LIMIT);
return new ImpactGraph(convertImpactGraphNode(impactTree.getRoot()), impactTree.getImpactPathsCount(), ImpactTree.IMPACT_PATHS_LIMIT);
}

private static ImpactGraphNode convertImpactGraphNode(ImpactTreeNode impactTreeNode) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/jfrog/ide/idea/TestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static PsiElement getNonLeafElement(PsiFile fileDescriptor, Class<? exten
*/
public static DepTreeNode getAndAssertChild(DepTree depTree, DepTreeNode parent, String childName) {
Assert.assertTrue(parent.getChildren().contains(childName));
DepTreeNode childNode = depTree.getNodes().get(childName);
DepTreeNode childNode = depTree.nodes().get(childName);
Assert.assertNotNull("Couldn't find node '" + childName + "'.", childNode);
return childNode;
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/com/jfrog/ide/idea/scan/GradleScannerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void testBuildTree() throws IOException {
// Run and check scan results
DepTree results = gradleScanner.buildTree();
assertNotNull(results);
assertEquals(Paths.get(globalProjectDir).getFileName().toString(), results.getRootId());
assertEquals(Paths.get(globalProjectDir).getFileName().toString(), results.rootId());
assertEquals(3, results.getRootNode().getChildren().size());

// Check module dependency
Expand Down
12 changes: 6 additions & 6 deletions src/test/java/com/jfrog/ide/idea/scan/PypiScannerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ public void testBuildTree() throws IOException {

// Check root SDK node
DepTree results = pypiScanner.buildTree();
assertEquals(SDK_NAME, results.getRootId());
assertEquals(SDK_NAME, results.getRootId());
assertEquals(pythonSdk.getHomePath(), results.getRootNode().getDescriptorFilePath());
assertEquals(SDK_NAME, results.rootId());
assertEquals(SDK_NAME, results.rootId());
assertEquals(pythonSdk.getHomePath(), results.getRootNodeDescriptorFilePath());
assertNotEmpty(results.getRootNode().getChildren());

// Check direct dependency
Expand All @@ -124,7 +124,7 @@ public void testBuildTree() throws IOException {

// Check transitive dependency
String anyTreeDepId = pipGrip.getChildren().stream().filter(childId -> childId.startsWith(TRANSITIVE_DEPENDENCY_NAME + ":")).findFirst().get();
DepTreeNode anyTreeDepNode = results.getNodes().get(anyTreeDepId);
DepTreeNode anyTreeDepNode = results.nodes().get(anyTreeDepId);
assertNotNull("Couldn't find node '" + anyTreeDepId + "'.", anyTreeDepNode);
assertSize(1, anyTreeDepNode.getChildren());
}
Expand All @@ -136,8 +136,8 @@ public void testBuildTreeCircularDependency() throws IOException {

// Check root SDK node
DepTree results = pypiScanner.buildTree();
assertEquals(SDK_NAME, results.getRootId());
assertEquals(pythonSdk.getHomePath(), results.getRootNode().getDescriptorFilePath());
assertEquals(SDK_NAME, results.rootId());
assertEquals(pythonSdk.getHomePath(), results.getRootNodeDescriptorFilePath());
assertNotEmpty(results.getRootNode().getChildren());

// The expected tree: root -> a-> b -> a
Expand Down
Loading

0 comments on commit afcd52b

Please sign in to comment.