Merge branch 'security-vulnerable-evidence-12-7' into 'master'

Fix Vulnerability of Release Evidence

Closes #12

See merge request gitlab-org/security/gitlab!38

app/controllers/projects/releases_controller.rb

@@ -10,7 +10,7 @@ class Projects::ReleasesController < Projects::ApplicationController
     push_frontend_feature_flag(:release_evidence_collection, project)
   end
   before_action :authorize_update_release!, only: %i[edit update]
-  before_action :authorize_download_code!, only: [:evidence]
+  before_action :authorize_read_release_evidence!, only: [:evidence]
 
   def index
     respond_to do |format|
@@ -47,6 +47,11 @@ class Projects::ReleasesController < Projects::ApplicationController
     access_denied! unless can?(current_user, :update_release, release)
   end
 
+  def authorize_read_release_evidence!
+    access_denied! unless Feature.enabled?(:release_evidence, project, default_enabled: true)
+    access_denied! unless can?(current_user, :read_release_evidence, release)
+  end
+
   def release
     @release ||= project.releases.find_by_tag!(sanitized_tag_name)
   end

app/models/evidence.rb

@@ -15,6 +15,21 @@ class Evidence < ApplicationRecord
     @milestones ||= release.milestones.includes(:issues)
   end
 
+  ##
+  # Return `summary` without sensitive information.
+  #
+  # Removing issues from summary in order to prevent leaking confidential ones.
+  # See more https://gitlab.com/gitlab-org/gitlab/issues/121930
+  def summary
+    safe_summary = read_attribute(:summary)
+
+    safe_summary.dig('release', 'milestones')&.each do |milestone|
+      milestone.delete('issues')
+    end
+
+    safe_summary
+  end
+
   private
 
   def generate_summary_and_sha

app/policies/release_policy.rb

@@ -2,4 +2,31 @@
 
 class ReleasePolicy < BasePolicy
   delegate { @subject.project }
+
+  rule { allowed_to_read_evidence & external_authorization_service_disabled }.policy do
+    enable :read_release_evidence
+  end
+
+  ##
+  # evidence.summary includes the following entities:
+  # - Release
+  # - git-tag (Repository)
+  # - Project
+  # - Milestones
+  # - Issues
+  condition(:allowed_to_read_evidence) do
+    can?(:read_release) &&
+      can?(:download_code) &&
+      can?(:read_project) &&
+      can?(:read_milestone) &&
+      can?(:read_issue)
+  end
+
+  ##
+  # Currently, we don't support release evidence for the GitLab instances
+  # that enables external authorization services.
+  # See https://gitlab.com/gitlab-org/gitlab/issues/121930.
+  condition(:external_authorization_service_disabled) do
+    !Gitlab::ExternalAuthorization::Config.enabled?
+  end
 end

changelogs/unreleased/security-vulnerable-evidence-12-7.yml

+---
+title: Fix Vulnerability of Release Evidence
+merge_request:
+author:
+type: security

lib/api/entities.rb

@@ -1363,7 +1363,7 @@ module API
       expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? }
       expose :commit, using: Entities::Commit, if: ->(_, _) { can_download_code? }
       expose :upcoming_release?, as: :upcoming_release
-      expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? }
+      expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? && can_read_milestone? }
       expose :commit_path, expose_nil: false
       expose :tag_path, expose_nil: false
       expose :evidence_sha, expose_nil: false, if: ->(_, _) { can_download_code? }
@@ -1389,6 +1389,10 @@ module API
       def can_download_code?
         Ability.allowed?(options[:current_user], :download_code, object.project)
       end
+
+      def can_read_milestone?
+        Ability.allowed?(options[:current_user], :read_milestone, object.project)
+      end
     end
 
     class Tag < Grape::Entity

spec/controllers/projects/releases_controller_spec.rb

@@ -167,7 +167,7 @@ describe Projects::ReleasesController do
   end
 
   describe 'GET #evidence' do
-    let(:tag_name) { "v1.1.0-evidence" }
+    let_it_be(:tag_name) { "v1.1.0-evidence" }
     let!(:release) { create(:release, :with_evidence, project: project, tag: tag_name) }
     let(:tag) { CGI.escape(release.tag) }
     let(:format) { :json }
@@ -220,6 +220,85 @@ describe Projects::ReleasesController do
         it_behaves_like 'successful request'
       end
     end
+
+    context 'when release is associated to a milestone which includes an issue' do
+      let_it_be(:project) { create(:project, :repository, :public) }
+      let_it_be(:issue) { create(:issue, project: project) }
+      let_it_be(:milestone) { create(:milestone, project: project, issues: [issue]) }
+      let_it_be(:release) { create(:release, project: project, tag: tag_name, milestones: [milestone]) }
+
+      before do
+        create(:evidence, release: release)
+      end
+
+      shared_examples_for 'does not show the issue in evidence' do
+        it do
+          subject
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['release']['milestones']
+            .all? { |milestone| milestone['issues'].nil? }).to eq(true)
+        end
+      end
+
+      shared_examples_for 'evidence not found' do
+        it do
+          subject
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      shared_examples_for 'safely expose evidence' do
+        it_behaves_like 'does not show the issue in evidence'
+
+        context 'when the issue is confidential' do
+          let(:issue) { create(:issue, :confidential, project: project) }
+
+          it_behaves_like 'does not show the issue in evidence'
+        end
+
+        context 'when the user is the author of the confidential issue' do
+          let(:issue) { create(:issue, :confidential, project: project, author: user) }
+
+          it_behaves_like 'does not show the issue in evidence'
+        end
+
+        context 'when project is private' do
+          let!(:project) { create(:project, :repository, :private) }
+
+          it_behaves_like 'evidence not found'
+        end
+
+        context 'when project restricts the visibility of issues to project members only' do
+          let!(:project) { create(:project, :repository, :issues_private) }
+
+          it_behaves_like 'evidence not found'
+        end
+      end
+
+      context 'when user is non-project member' do
+        let(:user) { create(:user) }
+
+        it_behaves_like 'safely expose evidence'
+      end
+
+      context 'when user is auditor', if: Gitlab.ee? do
+        let(:user) { create(:user, :auditor) }
+
+        it_behaves_like 'safely expose evidence'
+      end
+
+      context 'when external authorization control is enabled' do
+        let(:user) { create(:user) }
+
+        before do
+          stub_application_setting(external_authorization_service_enabled: true)
+        end
+
+        it_behaves_like 'evidence not found'
+      end
+    end
   end
 
   private

spec/fixtures/api/schemas/evidences/milestone.json

@@ -7,8 +7,7 @@
     "state",
     "iid",
     "created_at",
-    "due_date",
-    "issues"
+    "due_date"
   ],
   "properties": {
     "id": { "type": "integer" },
@@ -17,11 +16,7 @@
     "state": { "type": "string" },
     "iid": { "type": "integer" },
     "created_at": { "type": "date" },
-    "due_date": { "type": ["date", "null"] },
-    "issues": {
-      "type": "array",
-      "items": { "$ref": "issue.json" }
-    }
+    "due_date": { "type": ["date", "null"] }
   },
   "additionalProperties": false
 }

spec/requests/api/releases_spec.rb

@@ -340,6 +340,40 @@ describe API::Releases do
 
           expect(response).to have_gitlab_http_status(:ok)
         end
+
+        context 'when release is associated to a milestone' do
+          let!(:release) do
+            create(:release, tag: 'v0.1', project: project, milestones: [milestone])
+          end
+
+          let(:milestone) { create(:milestone, project: project) }
+
+          it 'exposes milestones' do
+            get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+            expect(json_response['milestones'].first['title']).to eq(milestone.title)
+          end
+
+          context 'when project restricts visibility of issues and merge requests' do
+            let!(:project) { create(:project, :repository, :public, :issues_private, :merge_requests_private) }
+
+            it 'does not expose milestones' do
+              get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+              expect(json_response['milestones']).to be_nil
+            end
+          end
+
+          context 'when project restricts visibility of issues' do
+            let!(:project) { create(:project, :repository, :public, :issues_private) }
+
+            it 'exposes milestones' do
+              get api("/projects/#{project.id}/releases/v0.1", non_project_member)
+
+              expect(json_response['milestones'].first['title']).to eq(milestone.title)
+            end
+          end
+        end
       end
     end
   end