Merge branch '6391_fix_url_to_file_from_vulnerability' into 'master'

Fix parsing of vulnerabilities file lines

See merge request gitlab-org/gitlab-ee!6100

ee/app/assets/javascripts/vue_shared/security_reports/store/utils.js

@@ -47,9 +47,9 @@ function enrichVulnerabilityWithfeedback(vulnerability, feedback = []) {
  */
 function fileUrl(location, pathPrefix) {
   let lineSuffix = '';
-  if (!_.isEmpty(location.start_line)) {
+  if (location.start_line) {
     lineSuffix += `#L${location.start_line}`;
-    if (!_.isEmpty(location.end_line)) {
+    if (location.end_line) {
       lineSuffix += `-${location.end_line}`;
     }
   }
@@ -82,7 +82,7 @@ function adaptDeprecatedFormat(issue) {
     // Add the new location property
     location: {
       file: adapted.file,
-      start_line: adapted.line,
+      start_line: adapted.line ? parseInt(adapted.line, 10) : undefined,
     },
     links,
   });

spec/javascripts/vue_shared/security_reports/mock_data.js

@@ -46,6 +46,8 @@ export const sastIssues = [
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
     location: {
       file: 'Gemfile.lock',
+      start_line: 5,
+      end_line: 10,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
@@ -106,6 +108,7 @@ export const oldSastIssues = [
     url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
     cve: 'CVE-2014-7829',
     file: 'Gemfile.lock',
+    line: '5',
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
   },
 ];
@@ -120,6 +123,8 @@ export const sastIssuesBase = [
       'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
     location: {
       file: 'Gemfile.lock',
+      start_line: 5,
+      end_line: 10,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
@@ -161,11 +166,13 @@ export const parsedSastIssuesStore = [
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
     title: 'Arbitrary file existence disclosure in Action Pack',
     path: 'Gemfile.lock',
-    urlPath: 'path/Gemfile.lock',
+    urlPath: 'path/Gemfile.lock#L5-10',
     category: 'sast',
     project_fingerprint: 'f55331d66fd4f3bfb4237d48e9c9fa8704bd33c6',
     location: {
       file: 'Gemfile.lock',
+      start_line: 5,
+      end_line: 10,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
@@ -235,11 +242,13 @@ export const parsedSastIssuesHead = [
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
     title: 'Arbitrary file existence disclosure in Action Pack',
     path: 'Gemfile.lock',
-    urlPath: 'path/Gemfile.lock',
+    urlPath: 'path/Gemfile.lock#L5-10',
     category: 'sast',
     project_fingerprint: 'f55331d66fd4f3bfb4237d48e9c9fa8704bd33c6',
     location: {
       file: 'Gemfile.lock',
+      start_line: 5,
+      end_line: 10,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
@@ -286,11 +295,13 @@ export const parsedSastBaseStore = [
     solution:
       'upgrade to >= 5.0.0.beta1.1, >= 4.2.5.1, ~> 4.2.5, >= 4.1.14.1, ~> 4.1.14, ~> 3.2.22.1',
     path: 'Gemfile.lock',
-    urlPath: 'path/Gemfile.lock',
+    urlPath: 'path/Gemfile.lock#L5-10',
     category: 'sast',
     project_fingerprint: '3f5608c99f0c7442ba59bc6c0c1864d0000f8e1a',
     location: {
       file: 'Gemfile.lock',
+      start_line: 5,
+      end_line: 10,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/335P1DcLG00',
@@ -311,6 +322,7 @@ export const dependencyScanningIssues = [
     url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
     cve: 'CVE-2014-7829',
     file: 'Gemfile.lock',
+    line: '5',
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
   },
   {
@@ -361,15 +373,16 @@ export const parsedDependencyScanningIssuesStore = [
     url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
     cve: 'CVE-2014-7829',
     file: 'Gemfile.lock',
+    line: '5',
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
     title: 'Arbitrary file existence disclosure in Action Pack',
     path: 'Gemfile.lock',
-    urlPath: 'path/Gemfile.lock',
+    urlPath: 'path/Gemfile.lock#L5',
     category: 'dependency_scanning',
     project_fingerprint: 'f55331d66fd4f3bfb4237d48e9c9fa8704bd33c6',
     location: {
       file: 'Gemfile.lock',
-      start_line: undefined,
+      start_line: 5,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
@@ -426,15 +439,16 @@ export const parsedDependencyScanningIssuesHead = [
     url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',
     cve: 'CVE-2014-7829',
     file: 'Gemfile.lock',
+    line: '5',
     solution: 'upgrade to ~> 3.2.21, ~> 4.0.11.1, ~> 4.0.12, ~> 4.1.7.1, >= 4.1.8',
     title: 'Arbitrary file existence disclosure in Action Pack',
     path: 'Gemfile.lock',
-    urlPath: 'path/Gemfile.lock',
+    urlPath: 'path/Gemfile.lock#L5',
     category: 'dependency_scanning',
     project_fingerprint: 'f55331d66fd4f3bfb4237d48e9c9fa8704bd33c6',
     location: {
       file: 'Gemfile.lock',
-      start_line: undefined,
+      start_line: 5,
     },
     links: [{
       url: 'https://groups.google.com/forum/#!topic/rubyonrails-security/rMTQy4oRCGk',

spec/javascripts/vue_shared/security_reports/store/utils_spec.js

@@ -57,6 +57,9 @@ describe('security reports utils', () => {
       const parsed = parseSastIssues(oldSastIssues, [], 'path')[0];
       expect(parsed.title).toEqual(sastIssues[0].message);
       expect(parsed.path).toEqual(sastIssues[0].location.file);
+      expect(parsed.location.start_line).toEqual(sastIssues[0].location.start_line);
+      expect(parsed.location.end_line).toBeUndefined();
+      expect(parsed.urlPath).toEqual('path/Gemfile.lock#L5');
       expect(parsed.project_fingerprint).toEqual(sha1(sastIssues[0].cve));
     });
 
@@ -64,9 +67,17 @@ describe('security reports utils', () => {
       const parsed = parseSastIssues(sastIssues, [], 'path')[0];
       expect(parsed.title).toEqual(sastIssues[0].message);
       expect(parsed.path).toEqual(sastIssues[0].location.file);
+      expect(parsed.location.start_line).toEqual(sastIssues[0].location.start_line);
+      expect(parsed.location.end_line).toEqual(sastIssues[0].location.end_line);
+      expect(parsed.urlPath).toEqual('path/Gemfile.lock#L5-10');
       expect(parsed.project_fingerprint).toEqual(sha1(sastIssues[0].cve));
     });
 
+    it('generate correct path to file when there is no line', () => {
+      const parsed = parseSastIssues(sastIssues, [], 'path')[1];
+      expect(parsed.urlPath).toEqual('path/Gemfile.lock');
+    });
+
     it('includes vulnerability feedbacks', () => {
       const parsed = parseSastIssues(
         sastIssues,
@@ -85,9 +96,17 @@ describe('security reports utils', () => {
       const parsed = parseDependencyScanningIssues(dependencyScanningIssues, [], 'path')[0];
       expect(parsed.title).toEqual(dependencyScanningIssues[0].message);
       expect(parsed.path).toEqual(dependencyScanningIssues[0].file);
+      expect(parsed.location.start_line).toEqual(sastIssues[0].location.start_line);
+      expect(parsed.location.end_line).toBeUndefined();
+      expect(parsed.urlPath).toEqual('path/Gemfile.lock#L5');
       expect(parsed.project_fingerprint).toEqual(sha1(dependencyScanningIssues[0].cve));
     });
 
+    it('generate correct path to file when there is no line', () => {
+      const parsed = parseDependencyScanningIssues(dependencyScanningIssues, [], 'path')[1];
+      expect(parsed.urlPath).toEqual('path/Gemfile.lock');
+    });
+
     it('uses message to generate sha1 when cve is undefined', () => {
       const issuesWithoutCve = dependencyScanningIssues.map(issue => ({
         ...issue,