Add latest changes from gitlab-org/security/gitlab@13-0-stable-ee

3 files changed, 68 insertions, 41 deletions

diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index a24c0471d6c..8df4fc5e88c 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -463,6 +463,7 @@ class ProjectPolicy < BasePolicy
   rule { repository_disabled }.policy do
     prevent :push_code
     prevent :download_code
+    prevent :build_download_code
     prevent :fork_project
     prevent :read_commit_status
     prevent :read_pipeline
diff --git a/changelogs/unreleased/security-ci-job-token-has-access-to-private-files.yml b/changelogs/unreleased/security-ci-job-token-has-access-to-private-files.yml
new file mode 100644
index 00000000000..b96480de3c5
--- /dev/null
+++ b/changelogs/unreleased/security-ci-job-token-has-access-to-private-files.yml
@@ -0,0 +1,5 @@
+---
+title: Prevent fetching repository code with unauthorized ci token
+merge_request:
+author:
+type: security
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 09d54eb9df6..f91d5658626 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
 describe ProjectPolicy do
   include ExternalAuthorizationServiceHelpers
   include_context 'ProjectPolicy context'
+  let_it_be(:other_user) { create(:user) }
   let_it_be(:guest) { create(:user) }
   let_it_be(:reporter) { create(:user) }
   let_it_be(:developer) { create(:user) }
@@ -163,7 +164,7 @@ describe ProjectPolicy do
     subject { described_class.new(owner, project) }
 
     it 'disallows all permissions when the feature is disabled' do
-      project.project_feature.update(merge_requests_access_level: ProjectFeature::DISABLED)
+      project.project_feature.update!(merge_requests_access_level: ProjectFeature::DISABLED)
 
       mr_permissions = [:create_merge_request_from, :read_merge_request,
                         :update_merge_request, :admin_merge_request,
@@ -215,7 +216,7 @@ describe ProjectPolicy do
       subject { described_class.new(owner, project) }
 
       before do
-        project.project_feature.update(builds_access_level: ProjectFeature::DISABLED)
+        project.project_feature.update!(builds_access_level: ProjectFeature::DISABLED)
       end
 
       context 'without metrics_dashboard_allowed' do
@@ -260,7 +261,7 @@ describe ProjectPolicy do
       subject { described_class.new(guest, project) }
 
       before do
-        project.project_feature.update(builds_access_level: ProjectFeature::PRIVATE)
+        project.project_feature.update!(builds_access_level: ProjectFeature::PRIVATE)
       end
 
       it 'disallows pipeline and commit_status permissions' do
@@ -275,50 +276,70 @@ describe ProjectPolicy do
   end
 
   context 'repository feature' do
-    subject { described_class.new(owner, project) }
+    let(:repository_permissions) do
+      [
+        :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
+        :create_build, :read_build, :update_build, :admin_build, :destroy_build,
+        :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
+        :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
+        :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
+        :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment,
+        :destroy_release, :download_code, :build_download_code
+      ]
+    end
+
+    context 'when user is a project member' do
+      subject { described_class.new(owner, project) }
 
-    before do
-      project.project_feature.update(repository_access_level: ProjectFeature::DISABLED)
-    end
+      context 'when it is disabled' do
+        before do
+          project.project_feature.update!(
+            repository_access_level: ProjectFeature::DISABLED,
+            merge_requests_access_level: ProjectFeature::DISABLED,
+            builds_access_level: ProjectFeature::DISABLED,
+            forking_access_level: ProjectFeature::DISABLED
+          )
+        end
 
-    context 'without metrics_dashboard_allowed' do
-      before do
-        project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED)
-      end
+        context 'without metrics_dashboard_allowed' do
+          before do
+            project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED)
+          end
 
-      it 'disallows all permissions when the feature is disabled' do
-        repository_permissions = [
-          :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
-          :create_build, :read_build, :update_build, :admin_build, :destroy_build,
-          :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
-          :create_environment, :read_environment, :update_environment, :admin_environment, :destroy_environment,
-          :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
-          :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment,
-          :destroy_release
-        ]
+          it 'disallows all permissions when the feature is disabled' do
+            expect_disallowed(*repository_permissions)
+          end
+        end
 
-        expect_disallowed(*repository_permissions)
+        context 'with metrics_dashboard_allowed' do
+          before do
+            project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+          end
+
+          it 'disallows all permissions but read_environment when the feature is disabled' do
+            expect_disallowed(*(repository_permissions - [:read_environment]))
+            expect_allowed(:read_environment)
+          end
+        end
       end
     end
 
-    context 'with metrics_dashboard_allowed' do
-      before do
-        project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
-      end
+    context 'when user is some other user' do
+      subject { described_class.new(other_user, project) }
 
-      it 'disallows all permissions when the feature is disabled' do
-        repository_permissions = [
-          :create_pipeline, :update_pipeline, :admin_pipeline, :destroy_pipeline,
-          :create_build, :read_build, :update_build, :admin_build, :destroy_build,
-          :create_pipeline_schedule, :read_pipeline_schedule, :update_pipeline_schedule, :admin_pipeline_schedule, :destroy_pipeline_schedule,
-          :create_environment, :update_environment, :admin_environment, :destroy_environment,
-          :create_cluster, :read_cluster, :update_cluster, :admin_cluster,
-          :create_deployment, :read_deployment, :update_deployment, :admin_deployment, :destroy_deployment,
-          :destroy_release
-        ]
+      context 'when access level is private' do
+        before do
+          project.project_feature.update!(
+            repository_access_level: ProjectFeature::PRIVATE,
+            merge_requests_access_level: ProjectFeature::PRIVATE,
+            builds_access_level: ProjectFeature::PRIVATE,
+            forking_access_level: ProjectFeature::PRIVATE
+          )
+        end
 
-        expect_disallowed(*repository_permissions)
-        expect_allowed(:read_environment)
+        it 'disallows all permissions' do
+          expect_disallowed(*repository_permissions)
+        end
       end
     end
   end
@@ -601,7 +622,7 @@ describe ProjectPolicy do
 
       context 'feature enabled' do
         before do
-          project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+          project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED)
         end
 
         context 'with reporter' do
@@ -665,7 +686,7 @@ describe ProjectPolicy do
 
       context 'feature enabled' do
         before do
-          project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::ENABLED)
+          project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::ENABLED)
         end
 
         context 'with reporter' do
@@ -750,7 +771,7 @@ describe ProjectPolicy do
 
     context 'feature disabled' do
       before do
-        project.project_feature.update(metrics_dashboard_access_level: ProjectFeature::DISABLED)
+        project.project_feature.update!(metrics_dashboard_access_level: ProjectFeature::DISABLED)
       end
 
       context 'with reporter' do