From b21db42e227a55a8f5df4d2511342e58abe44765 Mon Sep 17 00:00:00 2001 From: Robert May <rmay@gitlab.com> Date: Thu, 17 Jun 2021 00:10:09 +0000 Subject: [PATCH] Implement fork network mirroring rule [RUN ALL RSPEC] [RUN AS-IF-FOSS] --- app/models/project.rb | 15 +++++ .../block_external_fork_network_mirrors.yml | 8 +++ ee/app/models/ee/project.rb | 15 +++++ ee/spec/models/project_spec.rb | 55 +++++++++++++++++++ locale/gitlab.pot | 3 + spec/models/project_spec.rb | 39 +++++++++++++ 6 files changed, 135 insertions(+) create mode 100644 config/feature_flags/development/block_external_fork_network_mirrors.yml diff --git a/app/models/project.rb b/app/models/project.rb index 16f7ea14ce206..1f8e8b81015c6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -825,6 +825,21 @@ def ids_with_issuables_available_for(user) from_union([with_issues_enabled, with_merge_requests_enabled]).select(:id) end + + def find_by_url(url) + uri = URI(url) + + return unless uri.host == Gitlab.config.gitlab.host + + match = Rails.application.routes.recognize_path(url) + + return if match[:unmatched_route].present? + return if match[:namespace_id].blank? || match[:id].blank? + + find_by_full_path(match.values_at(:namespace_id, :id).join("/")) + rescue ActionController::RoutingError, URI::InvalidURIError + nil + end end def initialize(attributes = nil) diff --git a/config/feature_flags/development/block_external_fork_network_mirrors.yml b/config/feature_flags/development/block_external_fork_network_mirrors.yml new file mode 100644 index 0000000000000..81ff34a3d6e48 --- /dev/null +++ b/config/feature_flags/development/block_external_fork_network_mirrors.yml @@ -0,0 +1,8 @@ +--- +name: block_external_fork_network_mirrors +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/60735 +rollout_issue_url: +milestone: '14.0' +type: development +group: group::source code +default_enabled: false diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb index c1509ab87586d..9fa45d20d9cac 100644 --- a/ee/app/models/ee/project.rb +++ b/ee/app/models/ee/project.rb @@ -218,6 +218,7 @@ def lock_for_confirmation!(id) less_than: ::Gitlab::Pages::MAX_SIZE / 1.megabyte } validates :approvals_before_merge, numericality: true, allow_blank: true + validate :import_url_inside_fork_network, if: :import_url_changed? with_options if: :mirror? do validates :import_url, presence: true @@ -852,5 +853,19 @@ def latest_not_ingested_security_pipeline(only_successful) pipeline_scope.with_reports(::Ci::JobArtifact.security_reports).first || pipeline_scope.with_legacy_security_reports.first end + + # If the project is inside a fork network, the mirror URL must + # also belong to a member of that fork network + def import_url_inside_fork_network + return unless ::Feature.enabled?(:block_external_fork_network_mirrors, self, default_enabled: :yaml) + + if forked? + mirror_project = ::Project.find_by_url(import_url) + + unless mirror_project.present? && fork_network_projects.include?(mirror_project) + errors.add(:url, _("must be inside the fork network")) + end + end + end end end diff --git a/ee/spec/models/project_spec.rb b/ee/spec/models/project_spec.rb index 1a365a138e5bf..414913147cb65 100644 --- a/ee/spec/models/project_spec.rb +++ b/ee/spec/models/project_spec.rb @@ -2412,6 +2412,61 @@ def stub_find_remote_root_ref(project, ref:) expect(project.reload.import_url).to eq('http://user:pass@test.com') end end + + context 'project is inside a fork network' do + subject { project } + + let(:project) { create(:project, fork_network: fork_network) } + let(:fork_network) { create(:fork_network) } + + before do + stub_config_setting(host: 'gitlab.com') + end + + context 'feature flag is disabled' do + before do + stub_feature_flags(block_external_fork_network_mirrors: false) + project.import_url = "https://customgitlab.com/foo/bar.git" + end + + it { is_expected.to be_valid } + end + + context 'the project is the root of the fork network' do + before do + project.import_url = "https://customgitlab.com/foo/bar.git" + expect(fork_network).to receive(:root_project).and_return(project) + end + + it { is_expected.to be_valid } + end + + context 'the URL is inside the fork network' do + before do + project.import_url = "https://#{Gitlab.config.gitlab.host}/#{project.fork_network.root_project.full_path}.git" + end + + it { is_expected.to be_valid } + end + + context 'the URL is external but the project exists' do + it 'raises an error' do + project.import_url = "https://customgitlab.com/#{project.fork_network.root_project.full_path}.git" + project.validate + + expect(project.errors[:url]).to include('must be inside the fork network') + end + end + + context 'the URL is not inside the fork network' do + it 'raises an error' do + project.import_url = "https://customgitlab.com/foo/bar.git" + project.validate + + expect(project.errors[:url]).to include('must be inside the fork network') + end + end + end end describe '#add_import_job' do diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1d63a5fb16abf..81e6b8ab09db4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -39189,6 +39189,9 @@ msgstr "" msgid "must be greater than start date" msgstr "" +msgid "must be inside the fork network" +msgstr "" + msgid "must have a unique schedule, status, and elapsed time" msgstr "" diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index ebadbb617fc30..7eb02749f72e9 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1661,6 +1661,45 @@ def subject end end + describe '.find_by_url' do + subject { described_class.find_by_url(url) } + + let_it_be(:project) { create(:project) } + + before do + stub_config_setting(host: 'gitlab.com') + end + + context 'url is internal' do + let(:url) { "https://#{Gitlab.config.gitlab.host}/#{path}" } + + context 'path is recognised as a project path' do + let(:path) { project.full_path } + + it { is_expected.to eq(project) } + + it 'returns nil if the path detection throws an error' do + expect(Rails.application.routes).to receive(:recognize_path).with(url) { raise ActionController::RoutingError, 'test' } + + expect { subject }.not_to raise_error(ActionController::RoutingError) + expect(subject).to be_nil + end + end + + context 'path is not a project path' do + let(:path) { 'probably/missing.git' } + + it { is_expected.to be_nil } + end + end + + context 'url is external' do + let(:url) { "https://foo.com/bar/baz.git" } + + it { is_expected.to be_nil } + end + end + context 'repository storage by default' do let(:project) { build(:project) } -- GitLab