From 56975d1b33783eb4cf3dfd5ef933fcdda50ec41a Mon Sep 17 00:00:00 2001 From: Ilona Shishov Date: Wed, 27 Sep 2023 14:31:58 +0300 Subject: [PATCH] chore: remove effective-pom generation (#212) Signed-off-by: Ilona Shishov --- src/providers/pom.xml.ts | 119 +++++----------- src/server.ts | 54 ++------ test/aggregators.test.ts | 6 +- test/providers/pom.xml.test.ts | 239 ++++++++++++--------------------- test/vulnerability.test.ts | 2 +- 5 files changed, 131 insertions(+), 289 deletions(-) diff --git a/src/providers/pom.xml.ts b/src/providers/pom.xml.ts index 3364018b..aa89dcc4 100644 --- a/src/providers/pom.xml.ts +++ b/src/providers/pom.xml.ts @@ -6,16 +6,10 @@ import { VERSION_TEMPLATE } from '../utils'; export class DependencyProvider implements IDependencyProvider { private xmlDocAst: XMLDocument; - private originalDeps: Array; ecosystem: string; - constructor(originalContents: string, enforceVersions: boolean, public classes: Array = ['dependencies']) { + constructor(public classes: Array = ['dependencies']) { this.ecosystem = 'maven'; - const { cst, tokenVector } = parse(originalContents); - const originalXmlDocAst = buildAst(cst as DocumentCstNode, tokenVector); - if (originalXmlDocAst.rootElement) { - this.originalDeps = this.getXMLDependencies(originalXmlDocAst, enforceVersions); - } } private findRootNodes(document: XMLDocument, rootElementName: string): Array { @@ -37,7 +31,7 @@ export class DependencyProvider implements IDependencyProvider { this.xmlDocAst = buildAst(cst as DocumentCstNode, tokenVector); } - private mapToDependency(dependenciesNode: XMLElement[]): Array { + private mapToDependency(deps: XMLElement[]): Array { class PomDependency { public element: XMLElement; public groupId: XMLElement; @@ -55,121 +49,72 @@ export class DependencyProvider implements IDependencyProvider { return [this.groupId, this.artifactId].find(e => !e.textContents[0]?.text) === undefined; } - isValidWithVersion(): boolean { - // none should have a empty text. - return [this.groupId, this.artifactId, this.version].find(e => !e.textContents[0]?.text) === undefined; - } - }; - const toDependency = (resolved: PomDependency, original: PomDependency): Dependency => { + const toDependency = (d: PomDependency): Dependency => { const dep: IKeyValueEntry = new KeyValueEntry( - `${original.groupId.textContents[0].text}/${original.artifactId.textContents[0].text}`, - { line: original.element.position.startLine, column: original.element.position.startColumn } + `${d.groupId.textContents[0].text}/${d.artifactId.textContents[0].text}`, + { line: d.element.position.startLine, column: d.element.position.startColumn } ); dep.context_range = { - start: { line: original.element.position.startLine - 1, character: original.element.position.startColumn - 1 }, - end: { line: original.element.position.endLine - 1, character: original.element.position.endColumn } + start: { line: d.element.position.startLine - 1, character: d.element.position.startColumn - 1 }, + end: { line: d.element.position.endLine - 1, character: d.element.position.endColumn } }; - dep.value = new Variant(ValueType.String, resolved.version.textContents[0].text); - if (original.version) { - const versionVal = original.version.textContents[0]; + if (d.version && d.version.textContents.length > 0) { + dep.value = new Variant(ValueType.String, d.version.textContents[0].text); + const versionVal = d.version.textContents[0]; dep.value_position = { line: versionVal.position.startLine, column: versionVal.position.startColumn }; } else { + dep.value = new Variant(ValueType.String, ''); dep.value_position = { line: 0, column: 0 }; - dep.context = dependencyTemplate(original.element); + dep.context = dependencyTemplate(d.element); } return new Dependency(dep); }; - const dependencyTemplate = (original: XMLElement): string => { + const dependencyTemplate = (dep: XMLElement): string => { let template = ''; - let hasVersion = false; let idx = 0; - let margin = original.textContents[idx].text; - original.subElements.forEach(e => { - if (e.name === 'version') { - template += `${original.textContents[idx++].text}<${e.name}>${VERSION_TEMPLATE}`; - } else { - template += `${original.textContents[idx++].text}<${e.name}>${e.textContents[0].text}`; + let margin = dep.textContents[idx].text; + dep.subElements.forEach(e => { + if (e.name !== 'version') { + template += `${dep.textContents[idx++].text}<${e.name}>${e.textContents[0].text}`; } }); - if (!hasVersion) { - template += `${margin}${VERSION_TEMPLATE}`; - } - template += `${original.textContents[idx].text}`; + template += `${margin}${VERSION_TEMPLATE}`; + template += `${dep.textContents[idx].text}`; return template; }; - const getMapKey = (element: PomDependency): string => { - return `${element.groupId.textContents[0].text}/${element.artifactId.textContents[0].text}`; - }; + const purgeTestDeps = (nodes: XMLElement[]): Array => nodes + // no test dependencies + .filter(e => !e.subElements.find(e => (e.name === 'scope' && e.textContents[0]?.text === 'test'))) + .map(e => new PomDependency(e)); - const buildDependencyMap = (original: Array, resolved: Array): Array => { - let result = new Array(); - if (original) { - const visited = new Map(); - let resolvedIdx = 0; - original.forEach(o => { - const k = getMapKey(o); - let r = visited.get(k); - if(r === undefined) { - r = resolvedIdx++; - visited.set(k, r); - } - if(resolved[r] !== undefined) { - result.push(resolved[r]); - } else { - result.push(null); - } - }); - } - return result; - }; + const validDeps = purgeTestDeps(deps).filter(e => e.isValid()); - if (this.originalDeps) { - const toPomDep = (nodes: XMLElement[]): Array => nodes - // no test dependencies - .filter(e => !e.subElements.find(e => (e.name === 'scope' && e.textContents[0].text === 'test'))) - .map(e => new PomDependency(e)); - - const resolvedDeps = toPomDep(dependenciesNode).filter(e => e.isValidWithVersion()); - const origDeps = toPomDep(this.originalDeps).filter(e => e.isValid()); - - const resolvedMap = buildDependencyMap(origDeps, resolvedDeps); - const result = new Array(); - origDeps.forEach((d, idx) => { - if(resolvedMap[idx] !== null) { - result.push(toDependency(resolvedMap[idx], d)); - } - }); - return result; - } - return new Array(); + const result = new Array(); + validDeps.forEach((d) => { + result.push(toDependency(d)); + }); + return result; } async collect(contents: string): Promise> { this.parseXml(contents); - const deps = this.getXMLDependencies(this.xmlDocAst, true); + const deps = this.getXMLDependencies(this.xmlDocAst); return this.mapToDependency(deps); } - private getXMLDependencies(doc: XMLDocument, enforceVersions: boolean): Array { + private getXMLDependencies(doc: XMLDocument): Array { let validElementNames = ['groupId', 'artifactId']; - if(enforceVersions) { - validElementNames.push('version'); - } return this.findRootNodes(doc, 'dependencies') //must not be a dependency under dependencyManagement .filter(e => { const parentElement = e.parent as XMLElement | undefined; - - if (parentElement) { - return parentElement.name !== 'dependencyManagement'; - } - return true; + return parentElement?.name !== 'dependencyManagement'; }) .map(node => node.subElements) .flat(1) diff --git a/src/server.ts b/src/server.ts index 97c620a4..c3e6fbb5 100644 --- a/src/server.ts +++ b/src/server.ts @@ -200,7 +200,7 @@ const fetchVulnerabilities = async (fileType: string, reqData: any) => { 'EXHORT_MVN_PATH': globalSettings.mvnExecutable, 'EXHORT_NPM_PATH': globalSettings.npmExecutable, 'EXHORT_GO_PATH': globalSettings.goExecutable, - 'EXHORT_DEV_MODE': config.exhort_dev_mode, + 'EXHORT_DEV_MODE': config.exhort_dev_mode }; if (globalSettings.exhortSnykToken !== '') { options['EXHORT_SNYK_TOKEN'] = globalSettings.exhortSnykToken; @@ -233,7 +233,7 @@ const fetchVulnerabilities = async (fileType: string, reqData: any) => { } }; -const sendDiagnostics = async (diagnosticFilePath: string, originalContents: string, effectiveContents: string, provider: IDependencyProvider) => { +const sendDiagnostics = async (diagnosticFilePath: string, contents: string, provider: IDependencyProvider) => { // get dependencies from response before firing diagnostics. const getDepsAndRunPipeline = response => { @@ -256,7 +256,7 @@ const sendDiagnostics = async (diagnosticFilePath: string, originalContents: str let deps = null; try { const start = new Date().getTime(); - deps = await provider.collect(effectiveContents ? effectiveContents : originalContents); + deps = await provider.collect(contents); const end = new Date().getTime(); connection.console.log(`manifest parse took ${end - start} ms, found ${deps.length} deps`); } catch (error) { @@ -269,9 +269,7 @@ const sendDiagnostics = async (diagnosticFilePath: string, originalContents: str } // map dependencies - const regexVersion = new RegExp(/^(~|\^)?([a-zA-Z0-9]+\.)?([a-zA-Z0-9]+\.)?([a-zA-Z0-9]+\.)?([a-zA-Z0-9]+)$/); - const validPackages = provider.ecosystem === 'golang' ? deps : deps.filter(d => regexVersion.test(d.version.value.trim())); - const pkgMap = new DependencyMap(validPackages); + const pkgMap = new DependencyMap(deps); // init aggregator let packageAggregator = provider.ecosystem === 'maven' ? new MavenVulnerabilityAggregator(provider) : new NoopVulnerabilityAggregator(provider); @@ -282,7 +280,7 @@ const sendDiagnostics = async (diagnosticFilePath: string, originalContents: str const start = new Date().getTime(); // fetch vulnerabilities - const request = fetchVulnerabilities(path.basename(diagnosticFilePath), originalContents).then(getDepsAndRunPipeline); + const request = fetchVulnerabilities(path.basename(diagnosticFilePath), contents).then(getDepsAndRunPipeline); await request; // report results @@ -297,52 +295,16 @@ const sendDiagnostics = async (diagnosticFilePath: string, originalContents: str }); }; -function sendDiagnosticsWithEffectivePom(uri, originalContents: string) { - let tempTarget = uri.replace('file://', '').replaceAll('%20', ' ').replace('pom.xml', ''); - const effectivePomPath = path.join(tempTarget, 'target', 'effective-pom.xml'); - const tmpPomPath = path.join(tempTarget, 'target', 'in-memory-pom.xml'); - if (!fs.existsSync(path.dirname(tmpPomPath))) { - fs.mkdirSync(path.dirname(tmpPomPath), { recursive: true}); - } - fs.writeFile(tmpPomPath, originalContents, (error) => { - if (error) { - server.connection.sendNotification('caError', error); - } else { - try { - execSync(`${globalSettings.mvnExecutable} help:effective-pom -Doutput='${effectivePomPath}' --quiet -f '${tmpPomPath}'`); - try { - const effectiveContents = fs.readFileSync(effectivePomPath, 'utf8'); - sendDiagnostics(uri, originalContents, effectiveContents, new PomXml(originalContents, false)); - } catch (error) { - server.connection.sendNotification('caError', error.message); - } - } catch (error) { - // Ignore. Non parseable pom and fall back to original content - server.connection.sendNotification('caSimpleWarning', 'Full component analysis cannot be performed until the Pom is valid.'); - connection.console.info('Unable to parse effective pom. Cause: ' + error.message); - sendDiagnostics(uri, originalContents, null, new PomXml(originalContents, true)); - } finally { - if (fs.existsSync(tmpPomPath)) { - fs.rmSync(tmpPomPath); - } - if (fs.existsSync(effectivePomPath)) { - fs.rmSync(effectivePomPath); - } - } - } - }); -} - files.on(EventStream.Diagnostics, '^package\\.json$', (uri, name, contents) => { - sendDiagnostics(uri, contents, null, new PackageJson()); + sendDiagnostics(uri, contents, new PackageJson()); }); files.on(EventStream.Diagnostics, '^pom\\.xml$', (uri, name, contents) => { - sendDiagnosticsWithEffectivePom(uri, contents); + sendDiagnostics(uri, contents, new PomXml()); }); files.on(EventStream.Diagnostics, '^go\\.mod$', (uri, name, contents) => { - sendDiagnostics(uri, contents, null, new GoMod()); + sendDiagnostics(uri, contents, new GoMod()); }); diff --git a/test/aggregators.test.ts b/test/aggregators.test.ts index 4afd2750..04f64b74 100644 --- a/test/aggregators.test.ts +++ b/test/aggregators.test.ts @@ -43,7 +43,7 @@ describe('Noop vulnerability aggregator tests', () => { describe('Maven vulnerability aggregator tests', () => { it('Test Maven aggregator with one vulnerability', async () => { - let mavenVulnerabilityAggregator = new MavenVulnerabilityAggregator(new PomXml('', false)); + let mavenVulnerabilityAggregator = new MavenVulnerabilityAggregator(new PomXml()); // let vulnerability = new Vulnerability(dummyRange, 'pkg:maven/MockPkg@1.2.3', 0, null, '', '', null, ''); let vulnerability = new Vulnerability(dummyRange, 'pkg:maven/MockPkg@1.2.3', 0, ''); let aggVulnerability = mavenVulnerabilityAggregator.aggregate(vulnerability); @@ -62,7 +62,7 @@ describe('Maven vulnerability aggregator tests', () => { }); it('Test Maven aggregator with two identical vulnerability', async () => { - let mavenVulnerabilityAggregator = new MavenVulnerabilityAggregator(new PomXml('', false)); + let mavenVulnerabilityAggregator = new MavenVulnerabilityAggregator(new PomXml()); // let vulnerability1 = new Vulnerability(dummyRange, 'pkg:maven/MockPkg@1.2.3', 0, null, '', '', null, ''); let vulnerability1 = new Vulnerability(dummyRange, 'pkg:maven/MockPkg@1.2.3', 0, ''); let aggVulnerability1 = mavenVulnerabilityAggregator.aggregate(vulnerability1); @@ -77,7 +77,7 @@ describe('Maven vulnerability aggregator tests', () => { }); it('Test Maven aggregator with two different vulnerability', async () => { - let mavenVulnerabilityAggregator = new MavenVulnerabilityAggregator(new PomXml('', false)); + let mavenVulnerabilityAggregator = new MavenVulnerabilityAggregator(new PomXml()); // let vulnerability1 = new Vulnerability(dummyRange, 'pkg:maven/MockPkg1@1.2.3', 0, null, '', '', null, ''); let vulnerability1 = new Vulnerability(dummyRange, 'pkg:maven/MockPkg1@1.2.3', 0, ''); let aggVulnerability1 = mavenVulnerabilityAggregator.aggregate(vulnerability1); diff --git a/test/providers/pom.xml.test.ts b/test/providers/pom.xml.test.ts index 7d8348cd..24269919 100644 --- a/test/providers/pom.xml.test.ts +++ b/test/providers/pom.xml.test.ts @@ -8,7 +8,7 @@ describe('Maven pom.xml parser test', () => { it('tests pom.xml with empty string', async () => { const pom = ` `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps.length).equal(0); }); @@ -17,7 +17,7 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps.length).equal(0); }); @@ -28,9 +28,9 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps.length).equal(0); - }); + }); it('tests valid pom.xml', async () => { const pom = ` @@ -50,7 +50,7 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'foo/bar', position: { line: 10, column: 17 } }, version: { value: '2.4', position: { line: 13, column: 30 } }, @@ -58,25 +58,7 @@ describe('Maven pom.xml parser test', () => { }); it('highlights duplicate dependencies', async () => { - const effective = ` - - - - c - ab-cd - 2.3 - test - - - foo - bar - 2.4 - - - - `; - - const original = ` + const pom = ` @@ -98,7 +80,7 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(original, false).collect(effective); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'foo/bar', position: { line: 10, column: 17 } }, version: { value: '2.4', position: { line: 13, column: 30 } }, @@ -109,25 +91,7 @@ describe('Maven pom.xml parser test', () => { }); it('highlights duplicate dependencies when one has version', async () => { - const effective = ` - - - - c - ab-cd - 2.3 - test - - - foo - bar - 2.4 - - - - `; - - const original = ` + const pom = ` @@ -148,13 +112,13 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(original, false).collect(effective); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'foo/bar', position: { line: 10, column: 17 } }, version: { value: '2.4', position: { line: 13, column: 30 } }, },{ name: { value: 'foo/bar', position: { line: 15, column: 17 } }, - version: { value: '2.4', position: { line: 0, column: 0 } }, + version: { value: '', position: { line: 0, column: 0 } }, context: { value: ` foo bar @@ -175,25 +139,7 @@ describe('Maven pom.xml parser test', () => { }); it('highlights duplicate dependencies when none has version', async () => { - const effective = ` - - - - c - ab-cd - 2.3 - test - - - foo - bar - 2.4 - - - - `; - - const original = ` + const pom = ` @@ -213,10 +159,10 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(original, false).collect(effective); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'foo/bar', position: { line: 10, column: 17 } }, - version: { value: '2.4', position: { line: 0, column: 0 } }, + version: { value: '', position: { line: 0, column: 0 } }, context: { value: ` foo bar @@ -235,8 +181,9 @@ describe('Maven pom.xml parser test', () => { } },{ name: { value: 'foo/bar', position: { line: 14, column: 17 } }, - version: { value: '2.4', position: { line: 0, column: 0 } }, - context: { value: ` + version: { value: '', position: { line: 0, column: 0 } }, + context: { value: + ` foo bar __VERSION__ @@ -281,7 +228,7 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'plugins/a', position: { line: 5, column: 21 } }, version: { value: '2.3', position: { line: 8, column: 34 } } @@ -323,7 +270,7 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps.length).equal(0); }); @@ -332,34 +279,84 @@ describe('Maven pom.xml parser test', () => { - c + ab-cd - 2.3 - foo - bar - 2.4 + c + c - invalid - c + + + + `; + const deps = await new DependencyProvider().collect(pom); + expect(deps.length).equal(0); + }); + + it('tests pom.xml with invalid dependency versions', async () => { + const pom = ` + + + + c + ab-cd + + + + c + ab-cd `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'c/ab-cd', position: { line: 4, column: 17 } }, - version: { value: '2.3', position: { line: 7, column: 30 } } - }, { - name: { value: 'foo/bar', position: { line: 9, column: 17 } }, - version: { value: '2.4', position: { line: 12, column: 30 } } + version: { value: '', position: { line: 0, column: 0 } }, + context: { value: + ` + c + ab-cd + __VERSION__ + `, + range: { + end: { + character: 29, + line: 7 + }, + start: { + character: 16, + line: 3 + } + } + } + },{ + name: { value: 'c/ab-cd', position: { line: 9, column: 17 } }, + version: { value: '', position: { line: 0, column: 0 } }, + context: { value: + ` + c + ab-cd + __VERSION__ + `, + range: { + end: { + character: 30, + line: 11 + }, + start: { + character: 16, + line: 8 + } + } + } }]); }); @@ -396,8 +393,8 @@ describe('Maven pom.xml parser test', () => { - ` - const deps = await new DependencyProvider(pom, false).collect(pom); + `; + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'c/ab-cd', position: { line: 21, column: 17 } }, version: { value: '2.3', position: { line: 24, column: 30 } } @@ -408,7 +405,7 @@ describe('Maven pom.xml parser test', () => { }); it('tests pom.xml without version and with properties', async () => { - const original = ` + const pom = ` @@ -424,48 +421,16 @@ describe('Maven pom.xml parser test', () => { c ab-other - - c - - - `; - - const effective = ` - - - - c - ab-cd - 2.3 - - - foo - bar - 2.4 - - - c - ab-other - 1.2.3 - - - c - - - - - - `; - const deps = await new DependencyProvider(original, false).collect(effective); + const deps = await new DependencyProvider().collect(pom); expect(deps).is.eql([{ name: { value: 'c/ab-cd', position: { line: 4, column: 17 } }, version: { value: '2.3', position: { line: 7, column: 30 } } }, { name: { value: '${some.example}/${other.example}', position: { line: 9, column: 17 } }, - version: { value: '2.4', position: { line: 0, column: 0 } }, + version: { value: '', position: { line: 0, column: 0 } }, context: { value: ` \${some.example} \${other.example} @@ -484,7 +449,7 @@ describe('Maven pom.xml parser test', () => { } }, { name: { value: 'c/ab-other', position: { line: 13, column: 17 } }, - version: { value: '1.2.3', position: { line: 0, column: 0 } }, + version: { value: '', position: { line: 0, column: 0 } }, context: { value: ` c ab-other @@ -526,37 +491,7 @@ describe('Maven pom.xml parser test', () => { `; - const deps = await new DependencyProvider(pom, false).collect(pom); + const deps = await new DependencyProvider().collect(pom); expect(deps.length).equal(0); }); - - it('ignores versions when the effective pom is not valid', async () => { - const original = ` - - - - c - ab-cd - 2.3 - test - - - foo - bar - - - foo - baz - 2.4 - - - - `; - - const deps = await new DependencyProvider(original, true).collect(original); - expect(deps).is.eql([{ - name: { value: 'foo/baz', position: { line: 14, column: 17 } }, - version: { value: '2.4', position: { line: 17, column: 30 } } - }]); - }); }); diff --git a/test/vulnerability.test.ts b/test/vulnerability.test.ts index 6d5447cf..e0016e24 100644 --- a/test/vulnerability.test.ts +++ b/test/vulnerability.test.ts @@ -118,7 +118,7 @@ describe('Vulnerability tests', () => { 1, 'MockSeverity' ); - vulnerability.provider = new PomXml('', false); + vulnerability.provider = new PomXml(); const msg = "MockRef\nKnown security vulnerabilities: 1\nHighest severity: MockSeverity"; let expectedDiagnostic: Diagnostic = {