Merge branch 'sast_relative_broken_links_10435' into 'master'

Link to vulnerability location URL in the issue

See merge request gitlab-org/gitlab!27747

ee/app/controllers/projects/vulnerability_feedback_controller.rb

@@ -105,6 +105,7 @@ class Projects::VulnerabilityFeedbackController < Projects::ApplicationControlle
 
   def vulnerability_data_params_attributes
     %i[
+      blob_path
       category
       confidence
       count

ee/app/services/issues/create_from_vulnerability_data_service.rb

@@ -5,15 +5,12 @@ module Issues
     def execute
       return error("Can't create issue") unless can?(@current_user, :create_issue, @project)
 
-      vulnerability = case @params[:category]
-                      when 'sast', 'dependency_scanning', 'dast'
-                        Gitlab::Vulnerabilities::StandardVulnerability.new(params)
-                      when 'container_scanning'
-                        Gitlab::Vulnerabilities::ContainerScanningVulnerability.new(params)
+      begin
+        vulnerability = Gitlab::Vulnerabilities::Parser.fabricate(@params)
+      rescue Gitlab::Vulnerabilities::InvalidCategoryError
+        return error('Invalid vulnerability category')
       end
 
-      return error('Invalid vulnerability category') unless vulnerability
-
       issue_params = {
         title: "Investigate vulnerability: #{vulnerability.title}",
         description: render_description(vulnerability),
@@ -36,8 +33,10 @@ module Issues
     end
 
     def render_description(vulnerability)
-      view = ActionView::Base.new(ActionController::Base.view_paths, {})
-      view.render(file: 'vulnerabilities/issue_description.md.erb', locals: { vulnerability: vulnerability })
+      ApplicationController.render(
+        template: 'vulnerabilities/issue_description.md.erb',
+        locals: { vulnerability: vulnerability }
+      )
     end
   end
 end

ee/app/services/merge_requests/create_from_vulnerability_data_service.rb

@@ -3,9 +3,11 @@
 module MergeRequests
   class CreateFromVulnerabilityDataService < ::BaseService
     def execute
-      vulnerability = create_vulnerability
-
-      return error('Invalid vulnerability category') unless vulnerability
+      begin
+        vulnerability = Gitlab::Vulnerabilities::Parser.fabricate(@params)
+      rescue Gitlab::Vulnerabilities::InvalidCategoryError
+        return error('Invalid vulnerability category')
+      end
 
       title_slug = Gitlab::Utils.slugify(vulnerability.title)
       source_branch = "remediate/%s-%s" % [
@@ -40,15 +42,6 @@ module MergeRequests
 
     private
 
-    def create_vulnerability
-      case @params[:category]
-      when 'sast', 'dependency_scanning', 'dast'
-        Gitlab::Vulnerabilities::StandardVulnerability.new(@params)
-      when 'container_scanning'
-        Gitlab::Vulnerabilities::ContainerScanningVulnerability.new(@params)
-      end
-    end
-
     def create_patch(vulnerability, source_branch, target_branch)
       diffs = vulnerability.data[:remediations].map { |remediation| Base64.decode64(remediation[:diff]) }
       head_commit = project.repository.find_branch(target_branch).dereferenced_target

ee/changelogs/unreleased/sast_relative_broken_links_10435.yml

+---
+title: Use absolute URLs to ensure links to the source of SAST vulnerabilities resolve
+merge_request: 27747
+author:
+type: fixed

ee/lib/gitlab/vulnerabilities/parser.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Vulnerabilities
+    InvalidCategoryError = Class.new(StandardError)
+
+    module Parser
+      class << self
+        def fabricate(params)
+          raise ::Gitlab::Vulnerabilities::InvalidCategoryError unless valid_categories.key?(params[:category])
+
+          category = params[:category]
+
+          if standard_vulnerability? category
+            Gitlab::Vulnerabilities::StandardVulnerability.new(params)
+          else
+            Gitlab::Vulnerabilities::ContainerScanningVulnerability.new(params)
+          end
+        end
+
+        private
+
+        def valid_categories
+          ::Vulnerabilities::Feedback.categories
+        end
+
+        def standard_vulnerability?(category)
+          (valid_categories.keys - ['container_scanning']).include?(category)
+        end
+      end
+    end
+  end
+end

ee/lib/gitlab/vulnerabilities/standard_vulnerability.rb

@@ -3,6 +3,7 @@
 module Gitlab
   module Vulnerabilities
     class StandardVulnerability < BaseVulnerability
+      include RequestAwareEntity
       # Passthrough properties
       %i[
         severity
@@ -40,10 +41,16 @@ module Gitlab
         "#{file}:#{line}"
       end
 
+      def blob_path
+        return unless @data[:blob_path]
+
+        @data[:blob_path].gsub(/^\//, '')
+      end
+
       def location_link
-        return file unless line
+        return location_text unless blob_path
 
-        "#{file}#L#{line}"
+        "#{root_url}#{blob_path}"
       end
     end
   end

ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb

@@ -122,6 +122,7 @@ describe Projects::VulnerabilityFeedbackController do
           title: 'Predictable pseudorandom number generator',
           description: 'Description of Predictable pseudorandom number generator',
           tool: 'find_sec_bugs',
+          blob_path: '/group_path/project_path/-/blob/commitsha/subdir/src/main/App.java#L15',
           location: {
             file: 'subdir/src/main/java/com/gitlab/security_products/tests/App.java',
             start_line: '41'

ee/spec/lib/gitlab/vulnerabilities/parser_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Vulnerabilities::Parser do
+  describe '.fabricate' do
+    let(:params) do
+      {
+        target_branch: 'master'
+      }
+    end
+
+    subject { described_class.fabricate(params) }
+
+    context 'with standard categories' do
+      let(:categories) do
+        %w(
+        sast
+        dast
+        dependency_scanning
+        )
+      end
+
+      it 'returns a Standard Vulnerability' do
+        categories.each do |category|
+          params[:category] = category
+          expect(subject).to be_a(Gitlab::Vulnerabilities::StandardVulnerability)
+          expect(subject.target_branch).to eq('master')
+        end
+      end
+    end
+
+    context 'with container scanning as category' do
+      it 'returns a Scanning Vulnerability' do
+        params[:category] = 'container_scanning'
+
+        expect(subject).to be_a(Gitlab::Vulnerabilities::ContainerScanningVulnerability)
+        expect(subject.target_branch).to eq('master')
+      end
+    end
+
+    context 'with an invalid category' do
+      it 'raises an exception' do
+        params[:category] = 'foo'
+        expect { subject }.to raise_error(Gitlab::Vulnerabilities::InvalidCategoryError)
+      end
+    end
+  end
+end

ee/spec/lib/gitlab/vulnerabilities/standard_vulnerability_spec.rb

@@ -259,19 +259,19 @@ describe Gitlab::Vulnerabilities::StandardVulnerability do
   end
 
   describe '#location_link' do
-    context 'when line is nil' do
-      it 'returns a string with file' do
-        vulnerability = described_class.new(file: file)
+    context 'when blob_path is nil' do
+      it 'returns file path' do
+        vulnerability = described_class.new(file: "/foo/bar/-/blob/sha/#{file}")
 
-        expect(vulnerability.location_link).to eq file
+        expect(vulnerability.location_link).to eq "/foo/bar/-/blob/sha/#{file}"
       end
     end
 
     context 'when line is present' do
       it 'returns a string with file and line' do
-        vulnerability = described_class.new(file: file, line: line)
+        vulnerability = described_class.new(blob_path: "/foo/bar/-/blob/sha/#{file}#L#{line}")
 
-        expect(vulnerability.location_link).to eq "#{file}#L#{line}"
+        expect(vulnerability.location_link).to eq "http://localhost/foo/bar/-/blob/sha/#{file}#L#{line}"
       end
     end
   end

ee/spec/services/ee/issues/create_from_vulnerability_data_service_spec.rb

@@ -56,6 +56,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
       context 'when a description is present' do
         let(:params) do
           {
+            blob_path: '/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15',
             category: 'sast',
             severity: 'Low', confidence: 'High',
             solution: 'Please do something!',
@@ -98,7 +99,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
 
             * Severity: Low
             * Confidence: High
-            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:15](subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
+            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:15](http://localhost/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
 
             ### Solution:
 
@@ -123,6 +124,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
       context 'when a description is NOT present' do
         let(:params) do
           {
+            blob_path: '/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15',
             category: 'sast',
             severity: 'Low', confidence: 'High',
             solution: 'Please do something!',
@@ -142,7 +144,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
 
             * Severity: Low
             * Confidence: High
-            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:15](subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
+            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:15](http://localhost/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
 
             ### Solution:
 
@@ -158,6 +160,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
       context 'when a description is present' do
         let(:params) do
           {
+            blob_path: '/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15',
             category: 'dependency_scanning',
             severity: 'Low', confidence: 'High',
             solution: 'Please do something!',
@@ -199,7 +202,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
 
             * Severity: Low
             * Confidence: High
-            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:15](subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
+            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:15](http://localhost/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
 
             ### Solution:
 
@@ -224,6 +227,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
       context 'when a description is NOT present' do
         let(:params) do
           {
+            blob_path: '/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15',
             category: 'dependency_scanning',
             priority: 'Low', line: '41',
             severity: 'Low', confidence: 'High',
@@ -243,7 +247,7 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
 
             * Severity: Low
             * Confidence: High
-            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:41](subdir/src/main/java/com/gitlab/security_products/tests/App.java#L41)
+            * Location: [subdir/src/main/java/com/gitlab/security_products/tests/App.java:41](http://localhost/group_path/sub_group_path/project_path/-/blob/commitsha/subdir/src/main/java/com/gitlab/security_products/tests/App.java#L15)
 
             ### Solution:
 
@@ -402,5 +406,15 @@ describe Issues::CreateFromVulnerabilityDataService, '#execute' do
         expect(result[:message]).to eq('Invalid vulnerability category')
       end
     end
+
+    context 'when category is missing' do
+      let(:params) { {} }
+      let(:result) { described_class.new(project, user, params).execute }
+
+      it 'return expected error' do
+        expect(result[:status]).to eq(:error)
+        expect(result[:message]).to eq('Invalid vulnerability category')
+      end
+    end
   end
 end

ee/spec/services/vulnerability_feedback/create_service_spec.rb

@@ -19,6 +19,7 @@ describe VulnerabilityFeedback::CreateService, '#execute' do
         project_fingerprint: '418291a26024a1445b23fe64de9380cdcdfd1fa8',
         comment: 'a dismissal comment',
         vulnerability_data: {
+          blob_path: '/path/to/blob',
           category: 'sast',
           priority: 'Low', line: '41',
           file: 'subdir/src/main/java/com/gitlab/security_products/tests/App.java',