Commit e9dc8322 authored by Shinya Maeda's avatar Shinya Maeda Committed by Lin Jen-Shin

Fix Vulnerability of Release Evidence

This commit fixes the vulnerability of the release evidence
parent dfd44347
......@@ -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
......
......@@ -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
......
......@@ -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
---
title: Fix Vulnerability of Release Evidence
merge_request:
author:
type: security
......@@ -1360,7 +1360,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? }
......@@ -1386,6 +1386,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
......
......@@ -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
......
......@@ -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
}
......@@ -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
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment