Add latest changes from gitlab-org/security/gitlab@16-2-stable-ee
This commit is contained in:
parent
d359b87cf5
commit
46be8cb975
|
|
@ -32,7 +32,8 @@ module Ci
|
|||
config.content,
|
||||
project: project,
|
||||
user: current_user,
|
||||
sha: sha
|
||||
sha: sha,
|
||||
verify_project_sha: true
|
||||
).execute
|
||||
|
||||
result.valid? ? result.root_variables_with_prefill_data : {}
|
||||
|
|
|
|||
|
|
@ -39,6 +39,10 @@ module Gitlab
|
|||
def prohibited_tag_checks
|
||||
return if deletion?
|
||||
|
||||
unless Gitlab::GitRefValidator.validate(tag_name)
|
||||
raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
|
||||
end
|
||||
|
||||
if tag_name.start_with?("refs/tags/") # rubocop: disable Style/GuardClause
|
||||
raise GitAccess::ForbiddenError, ERROR_MESSAGES[:prohibited_tag_name]
|
||||
end
|
||||
|
|
|
|||
|
|
@ -28,6 +28,9 @@ module Gitlab
|
|||
def initialize(project:, current_user:, sha: nil)
|
||||
@project = project
|
||||
@current_user = current_user
|
||||
# If the `sha` is not provided, the default is the project's head commit (or nil). In such case, we
|
||||
# don't need to call `YamlProcessor.verify_project_sha!`, which prevents redundant calls to Gitaly.
|
||||
@verify_project_sha = sha.present?
|
||||
@sha = sha || project&.repository&.commit&.sha
|
||||
end
|
||||
|
||||
|
|
@ -77,6 +80,7 @@ module Gitlab
|
|||
Gitlab::Ci::YamlProcessor.new(content, project: @project,
|
||||
user: @current_user,
|
||||
sha: @sha,
|
||||
verify_project_sha: @verify_project_sha,
|
||||
logger: logger).execute
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -8,6 +8,8 @@
|
|||
module Gitlab
|
||||
module Ci
|
||||
class YamlProcessor
|
||||
include Gitlab::Utils::StrongMemoize
|
||||
|
||||
ValidationError = Class.new(StandardError)
|
||||
|
||||
def initialize(config_content, opts = {})
|
||||
|
|
@ -28,6 +30,8 @@ module Gitlab
|
|||
return Result.new(errors: ['Please provide content of .gitlab-ci.yml'])
|
||||
end
|
||||
|
||||
verify_project_sha! if verify_project_sha?
|
||||
|
||||
@ci_config = Gitlab::Ci::Config.new(@config_content, **@opts)
|
||||
|
||||
unless @ci_config.valid?
|
||||
|
|
@ -47,6 +51,15 @@ module Gitlab
|
|||
@opts[:project]
|
||||
end
|
||||
|
||||
def sha
|
||||
@opts[:sha]
|
||||
end
|
||||
|
||||
def verify_project_sha?
|
||||
@opts.delete(:verify_project_sha) || false
|
||||
end
|
||||
strong_memoize_attr :verify_project_sha?
|
||||
|
||||
def run_logical_validations!
|
||||
@stages = @ci_config.stages
|
||||
@jobs = @ci_config.normalized_jobs
|
||||
|
|
@ -185,6 +198,24 @@ module Gitlab
|
|||
def error!(message)
|
||||
raise ValidationError, message
|
||||
end
|
||||
|
||||
def verify_project_sha!
|
||||
return unless project && sha && project.repository_exists? && project.commit(sha)
|
||||
|
||||
unless project_ref_contains_sha?
|
||||
error!('Could not validate configuration. Config originates from external project')
|
||||
end
|
||||
end
|
||||
|
||||
def project_ref_contains_sha?
|
||||
# A 5-minute cache TTL is sufficient to prevent Gitaly load issues while also mitigating rare
|
||||
# use cases concerning stale data. For example, when an external commit gets merged into the
|
||||
# project, there may be at most a 5-minute window where the `sha` is still considered external.
|
||||
Rails.cache.fetch(['project', project.id, 'ref/contains/sha', sha], expires_in: 5.minutes) do
|
||||
repo = project.repository
|
||||
repo.branch_names_contains(sha, limit: 1).any? || repo.tag_names_contains(sha, limit: 1).any?
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -15,6 +15,12 @@ RSpec.describe Gitlab::Checks::TagCheck, feature_category: :source_code_manageme
|
|||
end
|
||||
|
||||
context "prohibited tags check" do
|
||||
it 'prohibits tags name that include refs/heads at the head' do
|
||||
allow(subject).to receive(:tag_name).and_return("refs/heads/foo")
|
||||
|
||||
expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "You cannot create a tag with a prohibited pattern.")
|
||||
end
|
||||
|
||||
it "prohibits tag names that include refs/tags/ at the head" do
|
||||
allow(subject).to receive(:tag_name).and_return("refs/tags/foo")
|
||||
|
||||
|
|
|
|||
|
|
@ -6,8 +6,9 @@ RSpec.describe Gitlab::Ci::Lint, feature_category: :pipeline_composition do
|
|||
let_it_be(:project) { create(:project, :repository) }
|
||||
let_it_be(:user) { create(:user) }
|
||||
|
||||
let(:lint) { described_class.new(project: project, current_user: user) }
|
||||
let(:sha) { nil }
|
||||
let(:ref) { project.default_branch }
|
||||
let(:lint) { described_class.new(project: project, current_user: user, sha: sha) }
|
||||
|
||||
describe '#validate' do
|
||||
subject { lint.validate(content, dry_run: dry_run, ref: ref) }
|
||||
|
|
@ -250,6 +251,59 @@ RSpec.describe Gitlab::Ci::Lint, feature_category: :pipeline_composition do
|
|||
|
||||
subject
|
||||
end
|
||||
|
||||
context 'when sha is provided' do
|
||||
let(:sha) { project.commit.sha }
|
||||
|
||||
it 'runs YamlProcessor with verify_project_sha: true' do
|
||||
expect(Gitlab::Ci::YamlProcessor)
|
||||
.to receive(:new)
|
||||
.with(content, a_hash_including(verify_project_sha: true))
|
||||
.and_call_original
|
||||
|
||||
subject
|
||||
end
|
||||
|
||||
it_behaves_like 'content is valid'
|
||||
|
||||
context 'when the sha is invalid' do
|
||||
let(:sha) { 'invalid-sha' }
|
||||
|
||||
it_behaves_like 'content is valid'
|
||||
end
|
||||
|
||||
context 'when the sha is from a fork' do
|
||||
include_context 'when a project repository contains a forked commit'
|
||||
|
||||
let(:sha) { forked_commit_sha }
|
||||
|
||||
context 'when a project ref contains the sha' do
|
||||
before do
|
||||
mock_branch_contains_forked_commit_sha
|
||||
end
|
||||
|
||||
it_behaves_like 'content is valid'
|
||||
end
|
||||
|
||||
context 'when a project ref does not contain the sha' do
|
||||
it 'returns an error' do
|
||||
expect(subject).not_to be_valid
|
||||
expect(subject.errors).to include(/Could not validate configuration/)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when sha is not provided' do
|
||||
it 'runs YamlProcessor with verify_project_sha: false' do
|
||||
expect(Gitlab::Ci::YamlProcessor)
|
||||
.to receive(:new)
|
||||
.with(content, a_hash_including(verify_project_sha: false))
|
||||
.and_call_original
|
||||
|
||||
subject
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when using dry run mode' do
|
||||
|
|
|
|||
|
|
@ -3449,6 +3449,88 @@ module Gitlab
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe 'verify project sha', :use_clean_rails_redis_caching do
|
||||
include_context 'when a project repository contains a forked commit'
|
||||
|
||||
let(:config) { YAML.dump(job: { script: 'echo' }) }
|
||||
let(:verify_project_sha) { true }
|
||||
let(:sha) { forked_commit_sha }
|
||||
|
||||
let(:processor) { described_class.new(config, project: project, sha: sha, verify_project_sha: verify_project_sha) }
|
||||
|
||||
subject { processor.execute }
|
||||
|
||||
shared_examples 'when the processor is executed twice consecutively' do |branch_names_contains_sha = false|
|
||||
it 'calls Gitaly only once for each ref type' do
|
||||
expect(repository).to receive(:branch_names_contains).once.and_call_original
|
||||
expect(repository).to receive(:tag_names_contains).once.and_call_original unless branch_names_contains_sha
|
||||
|
||||
2.times { processor.execute }
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a project branch contains the forked commit sha' do
|
||||
before_all do
|
||||
repository.add_branch(project.owner, 'branch1', forked_commit_sha)
|
||||
end
|
||||
|
||||
after(:all) do
|
||||
repository.rm_branch(project.owner, 'branch1')
|
||||
end
|
||||
|
||||
it { is_expected.to be_valid }
|
||||
|
||||
it_behaves_like 'when the processor is executed twice consecutively', true
|
||||
end
|
||||
|
||||
context 'when a project tag contains the forked commit sha' do
|
||||
before_all do
|
||||
repository.add_tag(project.owner, 'tag1', forked_commit_sha)
|
||||
end
|
||||
|
||||
after(:all) do
|
||||
repository.rm_tag(project.owner, 'tag1')
|
||||
end
|
||||
|
||||
it { is_expected.to be_valid }
|
||||
|
||||
it_behaves_like 'when the processor is executed twice consecutively'
|
||||
end
|
||||
|
||||
context 'when a project ref does not contain the forked commit sha' do
|
||||
it 'returns an error' do
|
||||
is_expected.not_to be_valid
|
||||
expect(subject.errors).to include(/Could not validate configuration/)
|
||||
end
|
||||
|
||||
it_behaves_like 'when the processor is executed twice consecutively'
|
||||
end
|
||||
|
||||
context 'when verify_project_sha option is false' do
|
||||
let(:verify_project_sha) { false }
|
||||
|
||||
it { is_expected.to be_valid }
|
||||
end
|
||||
|
||||
context 'when project is not provided' do
|
||||
let(:project) { nil }
|
||||
|
||||
it { is_expected.to be_valid }
|
||||
end
|
||||
|
||||
context 'when sha is not provided' do
|
||||
let(:sha) { nil }
|
||||
|
||||
it { is_expected.to be_valid }
|
||||
end
|
||||
|
||||
context 'when sha is invalid' do
|
||||
let(:sha) { 'invalid-sha' }
|
||||
|
||||
it { is_expected.to be_valid }
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
require 'spec_helper'
|
||||
|
||||
RSpec.describe BlobViewer::GitlabCiYml do
|
||||
RSpec.describe BlobViewer::GitlabCiYml, feature_category: :source_code_management do
|
||||
include FakeBlobHelpers
|
||||
include RepoHelpers
|
||||
|
||||
|
|
@ -13,18 +13,20 @@ RSpec.describe BlobViewer::GitlabCiYml do
|
|||
let(:blob) { fake_blob(path: '.gitlab-ci.yml', data: data) }
|
||||
let(:sha) { sample_commit.id }
|
||||
|
||||
subject { described_class.new(blob) }
|
||||
subject(:blob_viewer) { described_class.new(blob) }
|
||||
|
||||
describe '#validation_message' do
|
||||
it 'calls prepare! on the viewer' do
|
||||
expect(subject).to receive(:prepare!)
|
||||
subject(:validation_message) { blob_viewer.validation_message(project: project, sha: sha, user: user) }
|
||||
|
||||
subject.validation_message(project: project, sha: sha, user: user)
|
||||
it 'calls prepare! on the viewer' do
|
||||
expect(blob_viewer).to receive(:prepare!)
|
||||
|
||||
validation_message
|
||||
end
|
||||
|
||||
context 'when the configuration is valid' do
|
||||
it 'returns nil' do
|
||||
expect(subject.validation_message(project: project, sha: sha, user: user)).to be_nil
|
||||
expect(validation_message).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -32,7 +34,29 @@ RSpec.describe BlobViewer::GitlabCiYml do
|
|||
let(:data) { 'oof' }
|
||||
|
||||
it 'returns the error message' do
|
||||
expect(subject.validation_message(project: project, sha: sha, user: user)).to eq('Invalid configuration format')
|
||||
expect(validation_message).to eq('Invalid configuration format')
|
||||
end
|
||||
end
|
||||
|
||||
context 'when the sha is from a fork' do
|
||||
include_context 'when a project repository contains a forked commit'
|
||||
|
||||
let(:sha) { forked_commit_sha }
|
||||
|
||||
context 'when a project ref contains the sha' do
|
||||
before do
|
||||
mock_branch_contains_forked_commit_sha
|
||||
end
|
||||
|
||||
it 'returns nil' do
|
||||
expect(validation_message).to be_nil
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a project ref does not contain the sha' do
|
||||
it 'returns an error' do
|
||||
expect(validation_message).to match(/Could not validate configuration/)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -32,15 +32,50 @@ RSpec.describe Ci::ListConfigVariablesService,
|
|||
}
|
||||
end
|
||||
|
||||
let(:expected_result) do
|
||||
{
|
||||
'KEY1' => { value: 'val 1', description: 'description 1' },
|
||||
'KEY2' => { value: 'val 2', description: '' },
|
||||
'KEY3' => { value: 'val 3' },
|
||||
'KEY4' => { value: 'val 4' }
|
||||
}
|
||||
end
|
||||
|
||||
before do
|
||||
synchronous_reactive_cache(service)
|
||||
end
|
||||
|
||||
it 'returns variable list' do
|
||||
expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' })
|
||||
expect(result['KEY2']).to eq({ value: 'val 2', description: '' })
|
||||
expect(result['KEY3']).to eq({ value: 'val 3' })
|
||||
expect(result['KEY4']).to eq({ value: 'val 4' })
|
||||
it 'returns variables list' do
|
||||
expect(result).to eq(expected_result)
|
||||
end
|
||||
|
||||
context 'when the ref is a sha from a fork' do
|
||||
include_context 'when a project repository contains a forked commit'
|
||||
|
||||
before do
|
||||
allow_next_instance_of(Gitlab::Ci::ProjectConfig) do |instance|
|
||||
allow(instance).to receive(:exists?).and_return(true)
|
||||
allow(instance).to receive(:content).and_return(YAML.dump(ci_config))
|
||||
end
|
||||
end
|
||||
|
||||
let(:ref) { forked_commit_sha }
|
||||
|
||||
context 'when a project ref contains the sha' do
|
||||
before do
|
||||
mock_branch_contains_forked_commit_sha
|
||||
end
|
||||
|
||||
it 'returns variables list' do
|
||||
expect(result).to eq(expected_result)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when a project ref does not contain the sha' do
|
||||
it 'returns empty response' do
|
||||
expect(result).to eq({})
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,22 @@
|
|||
# frozen_string_literal: true
|
||||
|
||||
RSpec.shared_context 'when a project repository contains a forked commit' do
|
||||
include ProjectForksHelper
|
||||
|
||||
let_it_be(:project) { create(:project, :repository) }
|
||||
let_it_be(:repository) { project.repository }
|
||||
let_it_be(:forked_project) { fork_project(project, project.owner, repository: true) }
|
||||
|
||||
let_it_be(:forked_commit_sha) do
|
||||
forked_project.repository.create_file(project.owner, 'file.txt', 'file', message: 'test', branch_name: 'master')
|
||||
end
|
||||
|
||||
before_all do
|
||||
# Creating this MR moves the forked commit to the project repository
|
||||
create(:merge_request, source_project: forked_project, target_project: project)
|
||||
end
|
||||
|
||||
def mock_branch_contains_forked_commit_sha
|
||||
allow(repository).to receive(:branch_names_contains).with(forked_commit_sha, limit: 1).and_return(['branch1'])
|
||||
end
|
||||
end
|
||||
Loading…
Reference in New Issue