From eaea074f9f244801bfeb0092ac1a3a8516ea9aff Mon Sep 17 00:00:00 2001 From: Or Geva Date: Thu, 21 Sep 2023 14:09:05 +0300 Subject: [PATCH] Yarn - Fix incorrect impact graph --- .../dependenciesRoot/rootTree.ts | 48 ++++ .../dependenciesRoot/yarnTree.ts | 62 ++-- .../utils/dependencyUtils.ts | 47 +--- .../utils/yarnImpactGraph.ts | 264 ++++++++++++++++++ src/main/utils/yarnUtils.ts | 2 +- src/test/tests/dependencyUtils.test.ts | 16 +- 6 files changed, 365 insertions(+), 74 deletions(-) create mode 100644 src/main/treeDataProviders/utils/yarnImpactGraph.ts diff --git a/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/rootTree.ts b/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/rootTree.ts index 97b9df0f3..d7e4c29e7 100644 --- a/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/rootTree.ts +++ b/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/rootTree.ts @@ -6,6 +6,7 @@ import { PackageType } from '../../../types/projectType'; import { DependenciesTreeNode } from '../dependenciesTreeNode'; import { DependencyScanResults } from '../../../types/workspaceIssuesDetails'; import { Utils } from '../../../utils/utils'; +import { IImpactGraph, IImpactGraphNode } from 'jfrog-ide-webview'; export enum BuildTreeErrorType { NotInstalled = '[Not Installed]', @@ -13,6 +14,7 @@ export enum BuildTreeErrorType { } export class RootNode extends DependenciesTreeNode { + public static IMPACT_PATHS_LIMIT: number = 50; private _projectDetails: ProjectDetails; private _workspaceFolder: string; @@ -73,4 +75,50 @@ export class RootNode extends DependenciesTreeNode { } return result; } + + /** + * Retrieves the impact paths of all child components, recursively from a given root, + * The number of impact paths collected may be limited by the '{@link RootNode.IMPACT_PATHS_LIMIT}'. + * @param root - the root to get it's children impact + * @param componentWithIssue - the component to generate the impact path for it + * @param size - the total size of the impacted path + */ + public createImpactedGraph(vulnerableDependencyname: string, vulnerableDependencyversion: string): IImpactGraph { + return RootNode.collectPaths(vulnerableDependencyname + ':' + vulnerableDependencyversion, this.children, 0); + } + + private static collectPaths(vulnerableDependencyId: string, children: DependenciesTreeNode[], size: number) { + let impactPaths: IImpactGraphNode[] = []; + for (let child of children) { + if (impactPaths.find(node => node.name === child.componentId)) { + // Loop encountered + continue; + } + + if (child.componentId === vulnerableDependencyId) { + if (size < RootNode.IMPACT_PATHS_LIMIT) { + RootNode.appendDirectImpact(impactPaths, child.componentId); + } + size++; + } + + let indirectImpact: IImpactGraph = this.collectPaths(vulnerableDependencyId, child.children, size); + RootNode.appendIndirectImpact(impactPaths, child.componentId, indirectImpact); + size = indirectImpact.pathsCount ?? size; + } + return { root: { children: impactPaths }, pathsCount: size } as IImpactGraph; + } + + private static appendDirectImpact(impactPaths: IImpactGraphNode[], componentId: string): void { + impactPaths.push({ name: componentId } as IImpactGraphNode); + } + + private static appendIndirectImpact(impactPaths: IImpactGraphNode[], componentId: string, indirectImpact: IImpactGraph): void { + if (!!indirectImpact.root.children?.length) { + impactPaths.push({ + name: componentId, + children: indirectImpact.root.children + } as IImpactGraphNode); + } + } } diff --git a/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/yarnTree.ts b/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/yarnTree.ts index ee099c998..f4d24243a 100644 --- a/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/yarnTree.ts +++ b/src/main/treeDataProviders/dependenciesTree/dependenciesRoot/yarnTree.ts @@ -8,6 +8,8 @@ import { DependenciesTreeNode } from '../dependenciesTreeNode'; import { BuildTreeErrorType, RootNode } from './rootTree'; import { PackageType } from '../../../types/projectType'; import { LogManager } from '../../../log/logManager'; +import { IImpactGraph } from 'jfrog-ide-webview'; +import { YarnImpactGraphUtil } from '../../utils/yarnImpactGraph'; export class YarnTreeNode extends RootNode { private static readonly COMPONENT_PREFIX: string = 'npm://'; @@ -16,11 +18,11 @@ export class YarnTreeNode extends RootNode { super(workspaceFolder, PackageType.Yarn, parent); } - public refreshDependencies() { - let listResults: any; + public loadYarnDependencies() { + let results: any; try { - listResults = this.runYarnList(); - this.populateDependencyTree(this, listResults?.data?.trees); + results = this.runYarnList(); + this.loadYarnList(this, results?.data?.trees); } catch (error) { this._logManager.logError(error, false); this._logManager.logMessageAndToastErr( @@ -36,36 +38,48 @@ export class YarnTreeNode extends RootNode { this.label = this.projectDetails.name; } - private populateDependencyTree(dependencyTreeNode: DependenciesTreeNode, nodes: any[]) { - if (!nodes) { + /** @override */ + public createImpactedGraph(name: string, version: string): IImpactGraph { + return new YarnImpactGraphUtil(name, version, this.generalInfo.getComponentId(), this.workspaceFolder).create(); + } + + /** + * Parse and load all yarn list's dependencies into concert object. + * @param parent - Parent Yarn dependency that is loaded into object from 'yarn list'. + * @param children - Child dependency of parent in Yarn list form + * @returns + */ + private loadYarnList(parent: DependenciesTreeNode, children: any[]) { + if (!children) { return; } - for (let node of nodes) { + for (let node of children) { // Shadow dependencies does not always contain exact version, and therefore we should skip them. if (node.shadow) { continue; } - const scope: string = NpmUtils.getDependencyScope(node.name); - let lastIndexOfAt: number = node.name.lastIndexOf('@'); - let dependencyName: string = node.name.substring(0, lastIndexOfAt); - let dependencyVersion: string = node.name.substring(lastIndexOfAt + 1); - - let generalInfo: GeneralInfo = new GeneralInfo(dependencyName, dependencyVersion, scope !== '' ? [scope] : [], '', PackageType.Yarn); - let hasRealChildren: boolean = this.hasRealChildren(node.children); - let treeCollapsibleState: vscode.TreeItemCollapsibleState = hasRealChildren - ? vscode.TreeItemCollapsibleState.Collapsed - : vscode.TreeItemCollapsibleState.None; - let componentId: string = dependencyName + ':' + dependencyVersion; - this.projectDetails.addDependency(YarnTreeNode.COMPONENT_PREFIX + componentId); - - let child: DependenciesTreeNode = new DependenciesTreeNode(generalInfo, treeCollapsibleState, dependencyTreeNode); - child.dependencyId = YarnTreeNode.COMPONENT_PREFIX + componentId; - if (hasRealChildren) { - this.populateDependencyTree(child, node.children); + this.addDependency(parent, node); + if (this.hasRealChildren(node.children)) { + this.loadYarnList(parent, node.children); } } } + private extractDependencyInfo(node: any): string[] { + const scope: string = NpmUtils.getDependencyScope(node.name); + let lastIndexOfAt: number = node.name.lastIndexOf('@'); + let name: string = node.name.substring(0, lastIndexOfAt); + let version: string = node.name.substring(lastIndexOfAt + 1); + return [name, version, scope]; + } + + private addDependency(parent: DependenciesTreeNode, node: any): void { + const [dependencyName, dependencyVersion, scope] = this.extractDependencyInfo(node); + const generalInfo: GeneralInfo = new GeneralInfo(dependencyName, dependencyVersion, scope !== '' ? [scope] : [], '', PackageType.Yarn); + new DependenciesTreeNode(generalInfo, vscode.TreeItemCollapsibleState.None, parent).xrayId = + YarnTreeNode.COMPONENT_PREFIX + dependencyName + ':' + dependencyVersion; + } + /** * Return true if the child dependencies contain any non shadowed dependency. * @param childDependencies - Child dependencies at 'yarn list' results diff --git a/src/main/treeDataProviders/utils/dependencyUtils.ts b/src/main/treeDataProviders/utils/dependencyUtils.ts index 6b9364672..20a5d7917 100644 --- a/src/main/treeDataProviders/utils/dependencyUtils.ts +++ b/src/main/treeDataProviders/utils/dependencyUtils.ts @@ -12,7 +12,7 @@ import { MavenUtils } from '../../utils/mavenUtils'; import { NpmUtils } from '../../utils/npmUtils'; import { PypiUtils } from '../../utils/pypiUtils'; import { YarnUtils } from '../../utils/yarnUtils'; -import { IImpactGraph, IImpactGraphNode, ILicense } from 'jfrog-ide-webview'; +import { IImpactGraph, ILicense } from 'jfrog-ide-webview'; import { IssueTreeNode } from '../issuesTree/issueTreeNode'; import { FocusType } from '../../constants/contextKeys'; import { DependencyScanResults, ScanResults } from '../../types/workspaceIssuesDetails'; @@ -32,7 +32,6 @@ import { FileTreeNode } from '../issuesTree/fileTreeNode'; export class DependencyUtils { public static readonly FAIL_TO_SCAN: string = '[Fail to scan]'; - public static IMPACT_PATHS_LIMIT: number = 50; /** * Scan all the dependencies of a given package for security issues and populate the given data and view objects with the information. @@ -328,17 +327,18 @@ export class DependencyUtils { if (!issues) { return paths; } + for (let i: number = 0; i < issues.length; i++) { let issue: IVulnerability = issues[i]; for (let [componentId, component] of Object.entries(issue.components)) { - const childGraph: IImpactGraph = this.getChildrenGraph(descriptorGraph, component, 0); + const ImpactedPaths: IImpactGraph = descriptorGraph.createImpactedGraph(component.package_name, component.package_version); paths.set(issue.issue_id + componentId, { root: { name: this.getGraphName(descriptorGraph), - children: childGraph.root.children + children: ImpactedPaths.root.children }, - pathsCount: childGraph.pathsCount, - pathsLimit: DependencyUtils.IMPACT_PATHS_LIMIT + pathsCount: ImpactedPaths.pathsCount, + pathsLimit: RootNode.IMPACT_PATHS_LIMIT } as IImpactGraph); } } @@ -351,41 +351,6 @@ export class DependencyUtils { : descriptorGraph.componentId; } - /** - * Retrieves the impact paths of all child components, recursively from a given root, - * The number of impact paths collected may be limited by the '{@link DependencyUtils.IMPACT_PATHS_LIMIT}'. - * @param root - the root to get it's children impact - * @param componentWithIssue - the component to generate the impact path for it - * @param size - the total size of the impacted path - */ - private static getChildrenGraph(root: DependenciesTreeNode, componentWithIssue: IComponent, size: number): IImpactGraph { - let impactPaths: IImpactGraphNode[] = []; - for (let child of root.children) { - let impactChild: IImpactGraphNode | undefined = impactPaths.find(p => p.name === child.componentId); - if (!impactChild) { - if (child.componentId === componentWithIssue.package_name + ':' + componentWithIssue.package_version) { - // Direct impact - if (size < DependencyUtils.IMPACT_PATHS_LIMIT) { - impactPaths.push({ - name: child.componentId - } as IImpactGraphNode); - } - size++; - } - // indirect impact - let indirectImpact: IImpactGraph = this.getChildrenGraph(child, componentWithIssue, size); - if (!!indirectImpact.root.children?.length) { - impactPaths.push({ - name: child.componentId, - children: indirectImpact.root.children - } as IImpactGraphNode); - } - size = indirectImpact.pathsCount ?? size; - } - } - return { root: { children: impactPaths }, pathsCount: size } as IImpactGraph; - } - /** * Populate the provided issues data to the project node (view element) * @param projectNode - the project node that will be populated diff --git a/src/main/treeDataProviders/utils/yarnImpactGraph.ts b/src/main/treeDataProviders/utils/yarnImpactGraph.ts new file mode 100644 index 000000000..4b05660f2 --- /dev/null +++ b/src/main/treeDataProviders/utils/yarnImpactGraph.ts @@ -0,0 +1,264 @@ +import { IImpactGraph, IImpactGraphNode } from 'jfrog-ide-webview'; +import { RootNode } from '../dependenciesTree/dependenciesRoot/rootTree'; +import { ScanUtils } from '../../utils/scanUtils'; + +type YarnWhyItem = StepItem | InfoItem; + +/** + * Represents a step item in the Yarn "why" output. + */ +interface StepItem { + type: 'list'; + data: ListData; +} + +/** + * Represents an info item in the Yarn "why" output. + */ +interface InfoItem { + type: 'info'; + data: string; +} + +/** + * Represents data within a step item, specifically a list of reasons. + */ +interface ListData { + type: 'reasons'; + items: string[]; +} +/** + * Utility class for creating an impact graph based on Yarn package manager's "why" command output. + */ +export class YarnImpactGraphUtil { + /** + * Creates an instance of YarnImpactGraphUtil. + * @param dependencyName - The name of the impact dependency. + * @param dependencyVersion - The version of the impact dependency. + * @param projectName - The name of the project. + * @param workspaceFolder - The folder where the project is located. + */ + constructor(private dependencyName: string, private dependencyVersion: string, private projectName: string, private workspaceFolder: string) {} + + /** + * Creates and returns an impact graph based on Yarn's "why" command output. + * @returns An impact graph. + */ + public create(): IImpactGraph { + const dependencyChain: string[] | undefined = this.findDependencyChain(this.runYarnWhy()); + if (dependencyChain) { + return this.createImpactGraphFromChains(dependencyChain); + } + + return {} as IImpactGraph; + } + + /** + * Finds the dependency chain, aka, the path from the dependency to the root, based on the supplied Yarn "why" command output. + * The dependency chain may appear as a part of a text or in a list of reasons. + * + * Example 1 (Text): + * {"type":"info","data":"This module exists because \"jest-cli#istanbul-api#mkdirp\" depends on it."} + * + * Example 2 (List): + * {"type":"list","data":{"type":"reasons","items":["Specified in \"dependencies\"","Hoisted from \"jest-cli#node-notifier#minimist\"","Hoisted from \"jest-cli#sane#minimist\""]}} + * + * @param output - The Yarn "why" command output to analyze. + * @returns A list of vulnerable dependency chains to the root. + */ + private findDependencyChain(output: YarnWhyItem[]): string[] { + const startIndex: number | undefined = this.findDependencyPosition(this.dependencyVersion, output); + if (!startIndex) { + return []; + } + for (let i: number = startIndex + 1; i < output.length; i++) { + const item: YarnWhyItem = output[i]; + switch (item.type) { + case 'list': + return this.extractMultipleChain(item.data.items); + case 'info': + if (item.data.startsWith('This module exists because')) { + return this.extractMultipleChain([item.data]); + } + } + } + + return []; + } + + /** + * Dependency may present in multiple versions in yarn why output, therefore, finds the position of the specified version in the Yarn "why" command output. + * @param version - The version to search for. + * @param output - The Yarn "why" command output to search within. + * @returns The index of the found version or undefined if not found. + */ + private findDependencyPosition(version: string, output: YarnWhyItem[]): number | undefined { + for (let i: number = 0; i < output.length; i++) { + const item: YarnWhyItem = output[i]; + if (item.type === 'info' && item.data.includes(version)) { + return i; + } + } + return undefined; + } + + /** + * Extracts multiple dependency chains from a list raw dependency string. + * @param list - An array of strings representing raw dependency chains. + * @returns An array of extracted dependency chains. + * + * input - ["Specified in \"dependencies\"","Hoisted from \"jest-cli#node-notifier#minimist\"","Hoisted from \"jest-cli#sane#minimist\""] + * output - ["minimist","jest-cli#node-notifier#minimist","jest-cli#sane#minimist"] + */ + private extractMultipleChain(list: string[]): string[] { + const results: string[] = []; + list.forEach(item => { + const chain: string | undefined = this.extractChain(item); + if (chain) { + results.push(chain); + } + }); + return results; + } + + /** + * Extracts a single dependency chain from a raw dependency string. + * @param rawDependencyChain - The raw dependency chain string. + * @returns The extracted dependency chain or undefined if not found. + */ + private extractChain(rawDependencyChain: string): string | undefined { + if (rawDependencyChain.toLocaleLowerCase().includes('specified in')) { + return this.dependencyName; + } + // Extract the path from the dependency chain using quotes + const startIndex: number = rawDependencyChain.indexOf('"'); + const endIndex: number = rawDependencyChain.lastIndexOf('"'); + if (startIndex !== -1 && endIndex !== -1) { + return rawDependencyChain.substring(startIndex + 1, endIndex); + } + return; + } + + /** + * Creates an impact graph based on a list of dependency chains. + * @param chains - An array of dependency chains as strings. + * @returns An impact graph object. + */ + private createImpactGraphFromChains(chains: string[]): IImpactGraph { + const trees: IImpactGraphNode[] = []; + for (let index: number = 0; index < chains.length && index < RootNode.IMPACT_PATHS_LIMIT; index++) { + trees.push(this.createImpactGraphNodeFromChain(chains[index])); + } + return { + root: this.mergeAllTrees(trees), + pathsCount: Math.min(RootNode.IMPACT_PATHS_LIMIT, chains.length), + pathsLimit: RootNode.IMPACT_PATHS_LIMIT + } as IImpactGraph; + } + + /** + * Merges two impact graph trees into a single tree. + * @param root1 - The root of the first tree to be merged. + * @param root2 - The root of the second tree to be merged. + * @returns The merged impact graph tree. + */ + public mergeTrees(root1: IImpactGraphNode | null, root2: IImpactGraphNode | null): IImpactGraphNode | null { + if (!root1 && !root2) { + return null; + } + if (!root1) { + return root2; + } + if (!root2) { + return root1; + } + + // Create a merged node with the same name + const mergedNode: IImpactGraphNode = { name: root1.name }; + + // Merge the children recursively + if (root1.children && root2.children) { + const mergedChildren: IImpactGraphNode[] = []; + + for (const child1 of root1.children) { + const matchingChild2: IImpactGraphNode | undefined = root2.children.find(child2 => child2.name === child1.name); + + if (matchingChild2) { + const tmp: IImpactGraphNode | null = this.mergeTrees(child1, matchingChild2); + if (tmp) { + mergedChildren.push(tmp); + } + } else { + mergedChildren.push(child1); // If not found in root2, keep the child from root1 + } + } + + for (const child2 of root2.children) { + const matchingChild1: IImpactGraphNode | undefined = root1.children.find(child1 => child1.name === child2.name); + + if (!matchingChild1) { + mergedChildren.push(child2); // Add children from root2 that are not present in root1 + } + } + + mergedNode.children = mergedChildren; + } else if (root1.children) { + mergedNode.children = root1.children; + } else { + mergedNode.children = root2.children; + } + + return mergedNode; + } + + /** + * Merges multiple impact graph trees into a single tree. + * @param trees - An array of impact graph trees. + * @returns The merged impact graph tree. + */ + public mergeAllTrees(trees: IImpactGraphNode[]): IImpactGraphNode | null { + if (trees.length === 0) { + return null; + } + + let mergedTree: IImpactGraphNode | null = trees[0]; + + for (let i: number = 1; i < trees.length; i++) { + mergedTree = this.mergeTrees(mergedTree, trees[i]); + } + return mergedTree; + } + + /** + * Creates an impact graph node from a single dependency chain. + * @param chain - A single dependency chain as a string. + * @returns An impact graph node. + */ + private createImpactGraphNodeFromChain(chain: string): IImpactGraphNode { + const splitted: string[] = chain.split('#'); + let currentNode: IImpactGraphNode = { name: this.projectName }; + const root: IImpactGraphNode = currentNode; + for (const item of splitted) { + const child: IImpactGraphNode = { name: item }; + currentNode.children = [child]; + currentNode = child; + } + if (currentNode.name !== this.dependencyName) { + currentNode.children = [{ name: this.dependencyName + ':' + this.dependencyVersion }]; + } else { + currentNode.name = this.dependencyName + ':' + this.dependencyVersion; + } + return root; + } + + /** + * Executes the Yarn "why" command and parses its JSON output. + */ + private runYarnWhy(): YarnWhyItem[] { + const output: string = ScanUtils.executeCmd('yarn why --json --no-progress ' + this.dependencyName, this.workspaceFolder).toString(); + return output + .split('\n') + .filter(line => line.trim() !== '') + .map((line: string) => JSON.parse(line)); + } +} diff --git a/src/main/utils/yarnUtils.ts b/src/main/utils/yarnUtils.ts index 3ce513c2f..719b5f10c 100644 --- a/src/main/utils/yarnUtils.ts +++ b/src/main/utils/yarnUtils.ts @@ -74,7 +74,7 @@ export class YarnUtils { } checkCanceled(); - root.refreshDependencies(); + root.loadYarnDependencies(); } } diff --git a/src/test/tests/dependencyUtils.test.ts b/src/test/tests/dependencyUtils.test.ts index 3f7d7c866..ff52c73eb 100644 --- a/src/test/tests/dependencyUtils.test.ts +++ b/src/test/tests/dependencyUtils.test.ts @@ -83,7 +83,7 @@ describe('Dependency Utils Tests', () => { ] } as IImpactGraphNode, pathsCount: 2, - pathsLimit: DependencyUtils.IMPACT_PATHS_LIMIT + pathsLimit: RootNode.IMPACT_PATHS_LIMIT } as IImpactGraph); map.set('XRAY-191882' + 'C:2.0.0', { root: { @@ -91,7 +91,7 @@ describe('Dependency Utils Tests', () => { children: [{ name: 'C:2.0.0' } as IImpactGraphNode] }, pathsCount: 1, - pathsLimit: DependencyUtils.IMPACT_PATHS_LIMIT + pathsLimit: RootNode.IMPACT_PATHS_LIMIT } as IImpactGraph); // issue XRAY-94201, for components B:1.0.0 map.set('XRAY-94201' + 'B:1.0.0', { @@ -100,7 +100,7 @@ describe('Dependency Utils Tests', () => { children: [{ name: 'B:1.0.0' } as IImpactGraphNode] }, pathsCount: 1, - pathsLimit: DependencyUtils.IMPACT_PATHS_LIMIT + pathsLimit: RootNode.IMPACT_PATHS_LIMIT } as IImpactGraph); // issue XRAY-142007, for components [A:1.0.1, C:2.0.0] map.set('XRAY-142007' + 'A:1.0.1', { @@ -109,7 +109,7 @@ describe('Dependency Utils Tests', () => { children: [{ name: 'B:1.0.0', children: [{ name: 'A:1.0.1' } as IImpactGraphNode] } as IImpactGraphNode] }, pathsCount: 1, - pathsLimit: DependencyUtils.IMPACT_PATHS_LIMIT + pathsLimit: RootNode.IMPACT_PATHS_LIMIT } as IImpactGraph); map.set('XRAY-142007' + 'C:2.0.0', { root: { @@ -117,7 +117,7 @@ describe('Dependency Utils Tests', () => { children: [{ name: 'C:2.0.0' } as IImpactGraphNode] }, pathsCount: 1, - pathsLimit: DependencyUtils.IMPACT_PATHS_LIMIT + pathsLimit: RootNode.IMPACT_PATHS_LIMIT } as IImpactGraph); return map; } @@ -139,15 +139,15 @@ describe('Dependency Utils Tests', () => { describe('Limit impacted graph', () => { it('Set paths limit to 1', () => { - const ORIGIN_IMPACT_PATHS_LIMIT: number = DependencyUtils.IMPACT_PATHS_LIMIT; - DependencyUtils.IMPACT_PATHS_LIMIT = 1; + const ORIGIN_IMPACT_PATHS_LIMIT: number = RootNode.IMPACT_PATHS_LIMIT; + RootNode.IMPACT_PATHS_LIMIT = 1; let impactedTree: Map = DependencyUtils.createImpactedGraph(root, getGraphResponse('scanGraphVulnerabilities')); assert.equal(impactedTree.get('XRAY-191882A:1.0.0')?.pathsCount, 2); assert.equal(impactedTree.get('XRAY-191882A:1.0.0')?.root.children?.length, 1); - DependencyUtils.IMPACT_PATHS_LIMIT = ORIGIN_IMPACT_PATHS_LIMIT; + RootNode.IMPACT_PATHS_LIMIT = ORIGIN_IMPACT_PATHS_LIMIT; }); });