Show vulnerability location URL in the issue

The vulnerability location currently only shows
the path to the offending code, which doesn’t
work from outside GitLab. This MR updates the
vulnerability location to show the full URL.

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',