Commit 25960a7e authored by Yorick Peterse's avatar Yorick Peterse

Automate validating of security merge requests

This adds the ability to validate security merge requests that are
assigned to the release tools bot on dev.gitlab.org. Whenever a merge
request does not meet the requirements it is assigned back to the
author, and a note is created informing the author about what steps they
should take next.
parent 064ddf8a
......@@ -148,6 +148,17 @@ close-expired-qa-issues:
variables:
- $CLOSE_EXPIRED_QA_ISSUES
validate-security-merge-requests:
<<: *with-bundle
stage: automation
script:
- bundle exec rake validate_security_merge_requests
only:
refs:
- schedules
variables:
- $VALIDATE_SECURITY_MERGE_REQUESTS
# chatops ---------------------------------------------------------------------
chatops:
......
......@@ -11,6 +11,7 @@ gem 'rugged', '~> 0.27.0'
gem 'sentry-raven', '~> 2.7', require: false
gem 'merge_db_schema', '~> 0.1'
gem 'version_sorter', '~> 2.2.0'
gem 'parallel', '~> 1.14'
group :development, :test do
gem 'byebug'
......
......@@ -54,7 +54,7 @@ GEM
minitest (5.10.3)
multi_xml (0.6.0)
multipart-post (2.0.0)
parallel (1.12.0)
parallel (1.14.0)
parser (2.5.0.5)
ast (~> 2.4.0)
powerpack (0.1.1)
......@@ -142,6 +142,7 @@ DEPENDENCIES
gitlab (~> 4.10.0)
httparty (~> 0.16.0)
merge_db_schema (~> 0.1)
parallel (~> 1.14)
pry
rake
rspec (~> 3.7.0)
......
......@@ -275,3 +275,8 @@ task :freeze do
HTTParty.post(webhook_url, body: { payload: JSON.dump(text: message) })
end
desc 'Validate security merge requests'
task :validate_security_merge_requests do
ReleaseTools::Security::MergeRequestsValidator.new.execute
end
......@@ -24,6 +24,9 @@ require 'rugged'
require 'stringio'
require 'time'
require 'yaml'
require 'parallel'
require 'etc'
require 'set'
Dotenv.load
......@@ -108,6 +111,10 @@ require 'release_tools/time_util'
require 'release_tools/upstream_merge'
require 'release_tools/upstream_merge_request'
require 'release_tools/version_client'
require 'release_tools/security/client'
require 'release_tools/security/pipeline'
require 'release_tools/security/merge_requests_validator'
require 'release_tools/security/merge_request_validator'
unless ENV['TEST']
require 'sentry-raven'
......
# frozen_string_literal: true
module ReleaseTools
module Security
# A GitLab API client to use when validating security merge requests.
class Client
# The API endpoint to use for validating merge requests.
#
# This uses dev.gitlab.org opposed to gitlab.com, as security merge
# requests are submitted on dev.gitlab.org.
API_ENDPOINT = 'https://dev.gitlab.org/api/v4'
# The username of the release tools bot that is responsible for verifying
# security merge requests.
RELEASE_TOOLS_BOT_USERNAME = 'gitlab-release-tools-bot'
attr_reader :gitlab_client
def initialize
@gitlab_client = ::Gitlab.client(
endpoint: API_ENDPOINT,
private_token: ENV.fetch('RELEASE_BOT_DEV_TOKEN')
)
end
# @param [String] project The full project path, such as
# `gitlab-org/gitlab-ce`.
def open_security_merge_requests(project)
# We use a `per_page` of 100 so we get as many merge requests as
# posibble in a single request.
gitlab_client.merge_requests(
project,
per_page: 100,
labels: 'security',
state: 'opened',
assignee_id: release_tools_bot.id
)
.auto_paginate
end
# @return [Gitlab::ObjectifiedHash]
def release_tools_bot
@release_tools_bot ||= gitlab_client
.users(username: RELEASE_TOOLS_BOT_USERNAME)
.first
end
# @param [Integer|String] project_id
# @param [Integer] merge_request_iid
# @return [Gitlab::ObjectifiedHash]
def latest_merge_request_pipeline(project_id, merge_request_iid)
# Since this API does not allow sorting _and_ returns pipelines in an
# inconsistent order, we must sort this list manually.
gitlab_client
.merge_request_pipelines(project_id, merge_request_iid)
.auto_paginate
.sort_by(&:id)
.last
end
def method_missing(name, *args)
if gitlab_client.respond_to?(name)
gitlab_client.public_send(name, *args)
else
super
end
end
def respond_to_missing?(name, include_private = false)
gitlab_client.respond_to?(name, include_private)
end
end
end
end
# frozen_string_literal: true
module ReleaseTools
module Security
class MergeRequestValidator
attr_reader :errors
# A regular expression to use for extracting all pending tasks from the
# merge request description. This pattern will match the following:
#
# - [ ] Task name here
# * [ ] Task name here
PENDING_TASKS = /(\*|-)\s*\[\s+\]/
# A regular expression to use for extracting all tasks (pending or not)
# from the merge request description. This pattern will match the
# following:
#
# - [ ] Task name here
# * [ ] Task name here
# - [x] Task name here
# * [x] Task name here
ALL_TASKS = /(\*|-)\s*\[(\s+|x)\]/
# A regular expression to use to determine if the merge request was
# assigned to a reviewer.
APPROVED_TASK = /-\s*\[x\]\s*Assign to a reviewer/
# A regular expression used to determine if the target branch of a merge
# request is valid.
ALLOWED_TARGET_BRANCHES = /^(master|\d+-\d+-stable(-ee)?)$/
# @param [Gitlab::ObjectifiedHash] merge_request
# @param [ReleaseTools::Security::Client] client
def initialize(merge_request, client)
@merge_request = merge_request
@client = client
@errors = []
end
def validate
validate_pipeline_status
validate_merge_status
validate_work_in_progress
validate_pending_tasks
validate_milestone
validate_merge_request_template
validate_reviewed
validate_target_branch
end
def validate_pipeline_status
pipeline = Pipeline.latest_for_merge_request(@merge_request, @client)
if pipeline.nil?
error('Missing pipeline', <<~ERROR)
No pipeline could be found for this merge request. Security merge
requests must have a pipeline that passes before they can be merged.
ERROR
elsif pipeline.failed?
error('Failing pipeline', <<~ERROR)
The latest pipeline has one or more failing builds. Merge requests
can not be merged unless the pipeline has passed. Some builds are
allowed to fail on dev.gitlab.org, which currently includes the
following builds:
* #{Pipeline::ALLOWED_FAILURES.join("\n* ")}
ERROR
elsif !pipeline.passed?
# This covers pipelines that are skipped, still running, or in another
# unknown state.
error('Pending pipeline', <<~ERROR)
The latest pipeline did not pass, or is still running. Merge
requests should not be assigned to me until the pipeline(s) have
finished.
ERROR
end
end
def validate_merge_status
if @merge_request.merge_status == 'cannot_be_merged'
error('The merge request can not be merged', <<~ERROR)
This merge request can currently not be merged, likely due to merge
conflicts introduced by other (security) merge requests. Please
rebase this merge request with the target branch and resolve any
conflicts.
ERROR
end
end
def validate_work_in_progress
if @merge_request.title.start_with?('WIP')
error('The merge request is marked as a work in progress', <<~ERROR)
Work in progress merge requests will not be merged, so make sure to
resolve the WIP status before assigning this merge request back to
me.
ERROR
end
end
def validate_pending_tasks
if @merge_request.description.match?(PENDING_TASKS)
error('There are one or more pending tasks', <<~ERROR)
Before a security merge request can be merged, _all_ tasks must have
been completed. If a task is not applicable, you can either mark it
as completed or remove it.
ERROR
end
end
def validate_milestone
if @merge_request.milestone.nil?
error('The merge request does not have a milestone', <<~ERROR)
This merge request does not appear to have a milestone. Backports
must use the milestone of the version they target. Merge requests
targeting master should use the latest milestone.
ERROR
end
end
def validate_merge_request_template
unless @merge_request.description.match?(ALL_TASKS)
error('The Security Release template is not used', <<~ERROR)
This merge request does not contain any tasks to complete,
suggesting that the "Security Release" merge request template was
not used. Security merge requests must use this merge request
template.
ERROR
end
end
def validate_reviewed
unless @merge_request.description.match?(APPROVED_TASK)
error('The merge request is not reviewed', <<~ERROR)
This merge request appears to not have been reviewed. Make sure that
the following task is present and completed:
- [ ] Assign to a reviewer
ERROR
end
end
def validate_target_branch
unless @merge_request.target_branch.match?(ALLOWED_TARGET_BRANCHES)
error('The target branch is invalid', <<~ERROR)
Security merge requests must target `master`, or a stable branch
such as 11-8-stable (or 11-8-stable-ee for Enterprise Edition).
Security branches are no longer in use and should not be used as
target branches.
ERROR
end
end
# @param [String] summary
# @param [String] details
def error(summary, details)
@errors << <<~HTML
<details>
<summary><strong>#{summary}</strong></summary>
<br />
#{details}
</details>
HTML
end
end
end
end
# frozen_string_literal: true
module ReleaseTools
module Security
# Validating of multiple security merge requests across different projects.
class MergeRequestsValidator
PROJECTS_TO_VERIFY = %w[
gitlab/gitlabhq
gitlab/gitlab-ee
gitlab/gitaly
gitlab/gitlab-workhorse
gitlab/omnibus-gitlab
].freeze
ERROR_TEMPLATE = <<~TEMPLATE.strip
@%<author_username>s
This security merge request does not meet our requirements for
security merge requests. Please take the following steps to ensure
this merge request can be merged:
1. Resolve all the errors listed below
2. Mark this discussion as resolved
3. Assign the merge request back to @%<bot_username>s
## Errors
The following errors were detected:
%<errors>s
<hr>
<sub>
:robot: This is an automated message generated using the
[release tools project](https://gitlab.com/gitlab-org/release-tools/).
If you believe there is an error, please create an issue in the
release tools project.
</sub>
TEMPLATE
def initialize
@client = Client.new
end
# Validates all security merge requests, returning those that were valid.
#
# The valid merge requests are returned so that other code can use these
# MRs, for example by merging them.
def execute
valid = []
PROJECTS_TO_VERIFY.each do |project|
merge_requests = @client.open_security_merge_requests(project)
validated = Parallel
.map(merge_requests, in_threads: Etc.nprocessors) do |mr|
verify_merge_request(mr)
end
valid.concat(validated.compact)
end
valid
end
# @param [Gitlab::ObjectifiedHash] basic_mr
def verify_merge_request(basic_mr)
# Merge requests retrieved using the MR list API do not include all data
# we need, such as pipeline details. To work around this we must perform
# an additional request for every merge request to get this data.
mr = @client.merge_request(basic_mr.project_id, basic_mr.iid)
validator = MergeRequestValidator.new(mr, @client)
validator.validate
if validator.errors.any?
reassign_with_errors(mr, validator.errors)
nil
else
mr
end
end
# @param [Gitlab::ObjectifiedHash] mr
# @param [Array<String>] errors
def reassign_with_errors(mr, errors)
project_id = mr.project_id
iid = mr.iid
@client.create_merge_request_discussion(
project_id,
iid,
body: format(
ERROR_TEMPLATE,
{
author_username: mr.author.username,
bot_username: Client::RELEASE_TOOLS_BOT_USERNAME,
errors: errors.join("\n\n")
}
)
)
@client.update_merge_request(project_id, iid, assignee_id: mr.author.id)
end
end
end
end
# frozen_string_literal: true
module ReleaseTools
module Security
class Pipeline
# On dev.gitlab.org certain jobs will fail, such as EE specific line
# checking jobs. For security MRs we allow these jobs to fail, at least
# while we still use dev.gitlab.org for the security workflow.
ALLOWED_FAILURES = %w[
ee_compat_check
code_quality
ee-specific-lines-check
ee-files-location-check
review-build-cng
].freeze
# @param [Gitlab::ObjectifiedHash] merge_request
# @param [ReleaseTools::Security::Client] client
def self.latest_for_merge_request(merge_request, client)
raw_pipeline =
if merge_request.pipeline
merge_request.pipeline
else
# For reasons unknown, the API doesn't always include a pipeline in
# the merge request details. To work around this, we request the
# latest pipeline for the current MR from the API.
client.latest_merge_request_pipeline(
merge_request.project_id,
merge_request.iid
)
end
new(client, raw_pipeline) if raw_pipeline
end
# @param [ReleaseTools::Security::Client] client
# @param [Gitlab::ObjectifiedHash] raw_pipeline
def initialize(client, raw_pipeline)
@client = client
@raw_pipeline = raw_pipeline
end
def passed?
@raw_pipeline.status == 'success'
end
def failed?
return false unless @raw_pipeline.status == 'failed'
allowed_to_fail = allowed_failures
latest_jobs.any? do |job|
job.status == 'failed' &&
!ALLOWED_FAILURES.include?(job.name) &&
!allowed_to_fail.include?(job.name)
end
end
def allowed_failures
# When retrieving pipeline details the "allow_failure" field is not
# included, only when retrieving commit statuses is it included. This
# means we need to perform an additional API call to determine which
# builds are allowed to fail.
@client
.commit_status(
@raw_pipeline.project_id,
@raw_pipeline.sha,
per_page: 100
)
.auto_paginate
.select(&:allow_failure)
.map(&:name)
.to_set
end
def latest_jobs
# When requesting failed jobs, the API also includes jobs that were
# later retried and passed. This means we need to manually determine
# what jobs are the latest ones, then manually verify their statuses.
latest_per_name = @client
.pipeline_jobs(
@raw_pipeline.project_id,
@raw_pipeline.id,
per_page: 100
)
.auto_paginate
.each_with_object({}) do |job, hash|
latest = hash[job.name]
hash[job.name] = job if latest.nil? || latest.id < job.id
end
latest_per_name.values
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe ReleaseTools::Security::Client do
let(:client) { described_class.new }
describe '#open_security_merge_requests' do
it 'returns the open security merge requests' do
merge_request = double(:merge_request)
merge_requests = double(:merge_requests, auto_paginate: [merge_request])
allow(client)
.to receive(:release_tools_bot)
.and_return(double(:bot, id: 1))
allow(client.gitlab_client)
.to receive(:merge_requests)
.with(
'foo/foo',
per_page: 100,
labels: 'security',
state: 'opened',
assignee_id: 1
)
.and_return(merge_requests)
expect(client.open_security_merge_requests('foo/foo'))
.to eq([merge_request])
end
end
describe '#release_tools_bot' do
it 'returns the release tools bot' do
bot = double(:bot)
allow(client.gitlab_client)
.to receive(:users)
.with(username: described_class::RELEASE_TOOLS_BOT_USERNAME)
.and_return([bot])
expect(client.release_tools_bot).to eq(bot)
end
end
describe '#latest_merge_request_pipeline' do
it 'returns the latest pipeline for a merge request' do
mr1 = double(:merge_request, id: 1)
mr2 = double(:merge_request, id: 2)
merge_requests = double(:merge_requests, auto_paginate: [mr2, mr1])
allow(client.gitlab_client)
.to receive(:merge_request_pipelines)
.with(1, 2)
.and_return(merge_requests)
expect(client.latest_merge_request_pipeline(1, 2)).to eq(mr2)
end
end
describe '#method_missing' do
it 'delegates valid methods to the internal GitLab client' do
allow(client.gitlab_client)
.to receive(:users)
.with(username: 'foo')
client.users(username: 'foo')
expect(client.gitlab_client).to have_received(:users)
end
it 'raises NoMethodError when using invalid methods' do
expect { client.kittens }.to raise_error(NoMethodError)
end
end
describe '#respond_to?' do
it 'returns true for an existing method' do
expect(client.respond_to?(:users)).to eq(true)
end
it 'returns false for a non-existing method' do
expect(client.respond_to?(:kittens)).to eq(false)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe ReleaseTools::Security::MergeRequestValidator do
describe '#validate' do
it 'validates the merge request' do
# All these methods are tested separately, so we just want to make sure
# they're called from the `validate` method.
merge_request = double(:merge_request)
client = double(:client)
validator = described_class.new(merge_request, client)
validation_methods = %i[
validate_pipeline_status
validate_merge_status
validate_work_in_progress
validate_pending_tasks
validate_milestone
validate_merge_request_template
validate_reviewed
validate_target_branch
]
validation_methods.each do |method|
allow(validator).to receive(method)
end
validator.validate
validation_methods.each do |method|
expect(validator).to have_received(method)
end
end
end
describe '#validate_pipeline_status' do
let(:merge_request) { double(:merge_request) }
let(:client) { double(:client) }
let(:validator) { described_class.new(merge_request, client) }
it 'adds an error when no pipeline could be found' do
allow(ReleaseTools::Security::Pipeline)
.to receive(:latest_for_merge_request)
.with(merge_request, client)
.and_return(nil)
validator.validate_pipeline_status
expect(validator.errors.first).to include('Missing pipeline')
end
it 'adds an error when the pipeline failed' do
pipeline = double(:pipeline, failed?: true)
allow(ReleaseTools::Security::Pipeline)
.to receive(:latest_for_merge_request)
.with(merge_request, client)
.and_return(pipeline)
validator.validate_pipeline_status
expect(validator.errors.first).to include('Failing pipeline')
end
it 'adds an error when the pipeline is not yet finished' do
pipeline = double(:pipeline, failed?: false, passed?: false)
allow(ReleaseTools::Security::Pipeline)
.to receive(:latest_for_merge_request)
.with(merge_request, client)
.and_return(pipeline)
validator.validate_pipeline_status
expect(validator.errors.first).to include('Pending pipeline')
end
it 'does not add an error when the pipeline passed' do
pipeline = double(:pipeline, failed?: false, passed?: true)
allow(ReleaseTools::Security::Pipeline)
.to receive(:latest_for_merge_request)
.with(merge_request, client)
.and_return(pipeline)
validator.validate_pipeline_status
expect(validator.errors).to be_empty
end
end
describe '#validate_merge_status' do
it 'adds an error when the merge request can not be merged' do
merge_request = double(:merge_request, merge_status: 'cannot_be_merged')
client = double(:client)
validator = described_class.new(merge_request, client)
validator.validate_merge_status
expect(validator.errors.first)
.to include('The merge request can not be merged')
end
it 'does not add an error when the merge request can be merged' do
merge_request = double(:merge_request, merge_status: 'can_be_merged')