Prevent multiple project membership for security policy bot users

Users of type security policy bot have additional permissions as a project member. As a guest they can:

• access_git
• create_pipeline
• create_bot_pipeline
• build_download_code

This is okay because security policy bot users can only be created by security policies. However, it creates a hidden risk when a security policy bot user is invited as a guest to a different project. It will not be obvious that the bot user has more than guest permissions.

Implementation plan

We have to re-think the concept of the bot project relationship. Long term, it would be better to create a new access level instead of using guest access with additional permissions. With the new access level we can add a validation to Member to ensure uniqueness of user_id if the access level is security_policy_bot.

For the short term security fix, we can add a check to the Members::CreatorService:

Suggested diff

diff --git a/ee/app/services/ee/members/creator_service.rb b/ee/app/services/ee/members/creator_service.rb
index 2276964b68c2..75cd4be9a7de 100644
--- a/ee/app/services/ee/members/creator_service.rb
+++ b/ee/app/services/ee/members/creator_service.rb
@@ -19,6 +19,15 @@ def after_commit_tasks
         finish_onboarding_user
       end
 
+      override :commit_member
+      def commit_member
+        if security_policy_bot_is_member_of_other_project?
+          member.errors.add(:base, 'security policy bot users cannot be added to other projects')
+        else
+          super
+        end
+      end
+
       def finish_onboarding_user
         return unless ::Onboarding.user_onboarding_in_progress?(member.user)
         return unless finished_welcome_step?
@@ -29,6 +38,12 @@ def finish_onboarding_user
       def finished_welcome_step?
         member.user.role?
       end
+
+      def security_policy_bot_is_member_of_other_project?
+        return false unless member.user.security_policy_bot?
+
+        ::Member.exists?(user_id: member.user.id)
+      end
     end
   end
 end
diff --git a/ee/spec/services/ee/members/creator_service_spec.rb b/ee/spec/services/ee/members/creator_service_spec.rb
index 9fb029629b1e..53d9c2ad8d92 100644
--- a/ee/spec/services/ee/members/creator_service_spec.rb
+++ b/ee/spec/services/ee/members/creator_service_spec.rb
@@ -55,5 +55,29 @@
         end
       end
     end
+
+    context 'when user is a security_policy_bot' do
+      let_it_be(:user) { create(:user, :security_policy_bot) }
+      let_it_be(:project) { create(:project) }
+
+      subject { described_class.add_member(project, user, :guest) }
+
+      it 'adds a member' do
+        expect{ subject }.to change { Member.count }.by(1)
+      end
+
+      context 'when the user is already a member of another project' do
+        let_it_be(:other_project) { create(:project) }
+        let_it_be(:membership) { create(:project_member, :guest, source: other_project, user: user) }
+
+        it 'does not add a member' do
+          expect{ subject }.not_to change { Member.count }
+        end
+
+        it 'adds an error message to the member' do
+          expect(subject.errors.messages).to include(base: ['security policy bot users cannot be added to other projects'])
+        end
+      end
+    end
   end
 end