Prevent deletion of protected branches via security policies
Why are we doing this work
We need to prevent users from deleting protected branches that are affected by at least one scan result policy, in order to prevent scenarios like the following:
- User has branch
develop
- User wants to push code that violates a security policy
- Clone branch
develop
togoing-rogue
- Change default branch to any branch
- Delete branch
develop
- Change all pipeline jobs to use
going-rogue
as reference branch for deploying release- Push violating code into
going-rogue
- Pipelines run and create release artifacts.
- Rename branch
going-rogue
todevelop
- Change default branch to
develop
- Now the security violating code is in the repository.
- It could be caught in next security scan but artifacts of a release have been created. This is a roundabout way of circumventing policies but it is possible. It also leaves a trail of git commits to traceback who made those pipeline changes.
Based on Who can modify a protected branch, there is a caveat to preventing deletion of branches that we'll have to cover:
No one can delete a protected branch using Git commands, however, users with at least Maintainer role can delete a protected branch from the UI or API.
Relevant links
Non-functional requirements
-
Documentation: changes should be documented -
Feature flag: this feature should be released behind feature flag -
Performance: -
Testing:
Implementation plan
diff --git a/app/services/branches/delete_service.rb b/app/services/branches/delete_service.rb
index 6efbdd161a14..e396d784ca69 100644
--- a/app/services/branches/delete_service.rb
+++ b/app/services/branches/delete_service.rb
@@ -37,3 +37,5 @@ def unlock_artifacts(branch_name)
end
end
end
+
+Branches::DeleteService.prepend_mod
diff --git a/ee/app/services/ee/branches/delete_service.rb b/ee/app/services/ee/branches/delete_service.rb
new file mode 100644
index 000000000000..8ca6eb6723a1
--- /dev/null
+++ b/ee/app/services/ee/branches/delete_service.rb
@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+module EE
+ module Branches
+ module DeleteService
+ extend ::Gitlab::Utils::Override
+
+ override :execute
+ def execute(branch_name)
+ if ::ProtectedBranch.protected?(project, branch_name) && project.scan_result_policy_reads.blocking_protected_branches.exists?
+ return ServiceResponse.error(
+ message: 'Deleting protected branches is blocked by security policies',
+ http_status: 403)
+ end
+
+ super
+ end
+ end
+ end
+end
Edited by Andy Schoenen