diff --git a/ee/app/services/ee/branches/delete_service.rb b/ee/app/services/ee/branches/delete_service.rb index 8938c1fc314faeb2b0454aea4e4031003e62e29d..0118571ad90d5f0f53a29fb9831e622f5654bb16 100644 --- a/ee/app/services/ee/branches/delete_service.rb +++ b/ee/app/services/ee/branches/delete_service.rb @@ -25,7 +25,10 @@ def protect_branch_modification_is_blocked_by_security_policy?(project, branch_n return false unless project.licensed_feature_available?(:security_orchestration_policies) - service = ::Security::SecurityOrchestrationPolicies::ProtectedBranchesDeletionCheckService.new(project: project) + service = ::Security::SecurityOrchestrationPolicies::ProtectedBranchesDeletionCheckService.new( + project: project, + current_user: current_user + ) protected_from_deletion = service.execute([protected_branch]) protected_branch.in?(protected_from_deletion) diff --git a/ee/app/services/security/security_orchestration_policies/group_protected_branches_deletion_check_service.rb b/ee/app/services/security/security_orchestration_policies/group_protected_branches_deletion_check_service.rb index da2ddb950a96162f1594930e8b3d4889fab95d44..51bbf6aa8652668a2d4c5d28892cb7f42f58c2e4 100644 --- a/ee/app/services/security/security_orchestration_policies/group_protected_branches_deletion_check_service.rb +++ b/ee/app/services/security/security_orchestration_policies/group_protected_branches_deletion_check_service.rb @@ -15,13 +15,17 @@ def execute private def applicable_active_policies(config) - config - .active_scan_result_policies - .reject do |policy| - warn_mode_feature_enabled?(config) && policy[:enforcement_type] == ::Security::Policy::ENFORCEMENT_TYPE_WARN + policies = config.active_scan_result_policies + + policies.reject do |policy| + policy_in_warn_mode?(config, policy) || user_can_bypass_policy?(policy) end end + def policy_in_warn_mode?(config, policy) + warn_mode_feature_enabled?(config) && policy[:enforcement_type] == ::Security::Policy::ENFORCEMENT_TYPE_WARN + end + def applies?(policy) approval_settings = policy[:approval_settings] || (return false) @@ -47,6 +51,15 @@ def exceptions_permit_group?(setting) setting[:exceptions].all? { |exception| exception[:id] != group.id } end + def user_can_bypass_policy?(policy) + return false unless current_user + + Security::ScanResultPolicies::BranchDeletionBypassChecker.new( + policy: policy, + user: current_user + ).can_bypass? + end + def warn_mode_feature_enabled?(config) strong_memoize_with(:warn_mode_feature_enabled, config) do Feature.enabled?(:security_policy_approval_warn_mode, config.security_policy_management_project) diff --git a/ee/app/services/security/security_orchestration_policies/protected_branches_deletion_check_service.rb b/ee/app/services/security/security_orchestration_policies/protected_branches_deletion_check_service.rb index f9aa0f668da48f66e67cd0b933c8123ff6848164..5141d55f6982b9b88c317901c3e9eb7920121d0b 100644 --- a/ee/app/services/security/security_orchestration_policies/protected_branches_deletion_check_service.rb +++ b/ee/app/services/security/security_orchestration_policies/protected_branches_deletion_check_service.rb @@ -26,7 +26,7 @@ def blocked_branch_patterns def rules blocking_policies = applicable_scan_result_policies.select do |policy| - policy.dig(:approval_settings, :block_branch_modification) + policy.dig(:approval_settings, :block_branch_modification) && !user_can_bypass_policy?(policy) end blocking_policies.pluck(:rules).flatten # rubocop: disable CodeReuse/ActiveRecord -- blocking_policies is not expected to be an ActiveRecord::Relation but an Array @@ -43,6 +43,15 @@ def applicable_scan_result_policies .select { |policy| policy_scope_checker.policy_applicable?(policy) } end + def user_can_bypass_policy?(policy) + return false unless current_user + + Security::ScanResultPolicies::BranchDeletionBypassChecker.new( + policy: policy, + user: current_user + ).can_bypass? + end + def policy_in_warn_mode?(policy) warn_mode_feature_enabled? && policy[:enforcement_type] == Security::Policy::ENFORCEMENT_TYPE_WARN end diff --git a/ee/lib/security/scan_result_policies/branch_deletion_bypass_checker.rb b/ee/lib/security/scan_result_policies/branch_deletion_bypass_checker.rb new file mode 100644 index 0000000000000000000000000000000000000000..1eeea7ad20828e0d65b99fbd3754cc5160743820 --- /dev/null +++ b/ee/lib/security/scan_result_policies/branch_deletion_bypass_checker.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class BranchDeletionBypassChecker + include Gitlab::Utils::StrongMemoize + + # Result object to capture bypass details for audit logging + BypassResult = Struct.new(:allowed, :bypass_type, :identifier, keyword_init: true) do + def allowed? + allowed + end + end + + def initialize(policy:, user:) + @policy = policy + @user = user + end + + def can_bypass? + return false unless user + + bypass_with_service_account? || bypass_with_access_token? + end + + # Returns a BypassResult with details about the bypass for audit logging + def check_bypass + return BypassResult.new(allowed: false) unless user + + service_account_id = check_service_account_bypass + return build_bypass_result(:service_account, service_account_id) if service_account_id + + token_ids = check_access_token_bypass + return build_bypass_result(:access_token, token_ids) if token_ids + + BypassResult.new(allowed: false) + end + + private + + attr_reader :policy, :user + + def build_bypass_result(bypass_type, identifier) + BypassResult.new(allowed: true, bypass_type: bypass_type, identifier: identifier) + end + + def bypass_settings + BypassSettings.new(policy[:bypass_settings]) + end + strong_memoize_attr :bypass_settings + + def bypass_with_service_account? + check_service_account_bypass.present? + end + + def bypass_with_access_token? + check_access_token_bypass.present? + end + + def check_service_account_bypass + return unless user.service_account? + return unless bypass_settings.service_account_ids.include?(user.id) + + user.id + end + + def check_access_token_bypass + return unless user.project_bot? + + token_ids = bypass_settings.access_token_ids + return if token_ids.blank? + + matching_token_ids = user.personal_access_tokens.active.id_in(token_ids).pluck_primary_key + matching_token_ids.presence + end + end + end +end diff --git a/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb b/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb index 403a3f58de0528e4e82b3ff94700dec5aac922bd..72d95759c0efbc0b127fbd7852897827dfb23b50 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_auditor.rb @@ -30,6 +30,11 @@ def log_merge_request_bypass(merge_request, security_policy, reason) log_bypass_event(:merge_request, security_policy, audit_details, reason, merge_request) end + def log_branch_deletion_bypass(bypass_type, identifier) + audit_details = build_branch_deletion_audit_details(bypass_type, identifier) + log_bypass_event(bypass_type, identifier, audit_details) + end + private attr_reader :security_policy, :project, :user, :branch_name @@ -86,6 +91,20 @@ def build_merge_request_audit_details(merge_request, security_policy, reason) } end + def build_branch_deletion_audit_details(bypass_type, identifier) + { + bypass_type: bypass_type, + action: :branch_deletion + }.tap do |details| + case bypass_type + when :service_account + details[:service_account_id] = identifier + when :access_token + details[:access_token_ids] = Array.wrap(identifier) + end + end + end + def bypass_scope_details(user_bypass_scope) case user_bypass_scope when :group @@ -104,6 +123,10 @@ def audit_name_for_bypass_type(bypass_type) case bypass_type when :merge_request 'security_policy_merge_request_bypass' + when :branch_deletion_service_account + 'security_policy_service_account_branch_deletion_bypass' + when :branch_deletion_access_token + 'security_policy_access_token_branch_deletion_bypass' else "security_policy_#{bypass_type}_push_bypass" end diff --git a/ee/spec/lib/security/scan_result_policies/branch_deletion_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/branch_deletion_bypass_checker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..dc811a252d64a24042f2af1ca0650b53b50f7b27 --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/branch_deletion_bypass_checker_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::BranchDeletionBypassChecker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + + let(:bypass_settings) { {} } + let(:policy) do + { + name: 'Test Policy', + approval_settings: { block_branch_modification: true }, + bypass_settings: bypass_settings + } + end + + subject(:checker) { described_class.new(policy: policy, user: user) } + + describe '#can_bypass?' do + context 'when user is nil' do + let(:user) { nil } + + it { expect(checker.can_bypass?).to be false } + end + + context 'when user is a regular user' do + let(:user) { create(:user) } + + it { expect(checker.can_bypass?).to be false } + + context 'when user is in bypass_settings users' do + let(:bypass_settings) { { users: [{ id: user.id }] } } + + it { expect(checker.can_bypass?).to be false } + end + end + + context 'when user is a service account' do + let(:user) { create(:user, :service_account) } + + context 'when service account is not in bypass_settings' do + it { expect(checker.can_bypass?).to be false } + end + + context 'when service account is in bypass_settings' do + let(:bypass_settings) { { service_accounts: [{ id: user.id }] } } + + it { expect(checker.can_bypass?).to be true } + end + + context 'when a different service account is in bypass_settings' do + let(:other_service_account) { create(:user, :service_account) } + let(:bypass_settings) { { service_accounts: [{ id: other_service_account.id }] } } + + it { expect(checker.can_bypass?).to be false } + end + end + + context 'when user is a project bot (access token)' do + let(:user) { create(:user, :project_bot) } + let!(:personal_access_token) { create(:personal_access_token, user: user) } + + context 'when access token is not in bypass_settings' do + it { expect(checker.can_bypass?).to be false } + end + + context 'when access token is in bypass_settings' do + let(:bypass_settings) { { access_tokens: [{ id: personal_access_token.id }] } } + + it { expect(checker.can_bypass?).to be true } + end + + context 'when a different access token is in bypass_settings' do + let(:other_token) { create(:personal_access_token) } + let(:bypass_settings) { { access_tokens: [{ id: other_token.id }] } } + + it { expect(checker.can_bypass?).to be false } + end + + context 'when access token is revoked' do + let(:bypass_settings) { { access_tokens: [{ id: personal_access_token.id }] } } + + before do + personal_access_token.revoke! + end + + it { expect(checker.can_bypass?).to be false } + end + end + end + + describe '#check_bypass' do + context 'when user is nil' do + let(:user) { nil } + + it 'returns a result with allowed: false' do + result = checker.check_bypass + + expect(result.allowed?).to be false + expect(result.bypass_type).to be_nil + expect(result.identifier).to be_nil + end + end + + context 'when user is a service account in bypass_settings' do + let(:user) { create(:user, :service_account) } + let(:bypass_settings) { { service_accounts: [{ id: user.id }] } } + + it 'returns a result with service_account bypass type' do + result = checker.check_bypass + + expect(result.allowed?).to be true + expect(result.bypass_type).to eq(:service_account) + expect(result.identifier).to eq(user.id) + end + end + + context 'when user is a project bot with access token in bypass_settings' do + let(:user) { create(:user, :project_bot) } + let!(:personal_access_token) { create(:personal_access_token, user: user) } + let(:bypass_settings) { { access_tokens: [{ id: personal_access_token.id }] } } + + it 'returns a result with access_token bypass type' do + result = checker.check_bypass + + expect(result.allowed?).to be true + expect(result.bypass_type).to eq(:access_token) + expect(result.identifier).to contain_exactly(personal_access_token.id) + end + end + + context 'when user cannot bypass' do + let(:user) { create(:user) } + + it 'returns a result with allowed: false' do + result = checker.check_bypass + + expect(result.allowed?).to be false + expect(result.bypass_type).to be_nil + expect(result.identifier).to be_nil + end + end + end +end diff --git a/ee/spec/services/ee/branches/delete_service_spec.rb b/ee/spec/services/ee/branches/delete_service_spec.rb index 2b8b8166c238d01ecefa57a63446e640f6cff0f0..24ca4dc22ebe972249f6da63d33d466764118c4f 100644 --- a/ee/spec/services/ee/branches/delete_service_spec.rb +++ b/ee/spec/services/ee/branches/delete_service_spec.rb @@ -87,6 +87,84 @@ it_behaves_like 'a deleted branch' end + + context 'with service account bypass' do + let(:service_account) { create(:user, :service_account) } + + subject(:execute_service) { described_class.new(project, service_account).execute(protected_branch_name) } + + before_all do + project.add_developer(service_account) + end + + include_context 'with approval policy' do + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { service_accounts: [{ id: service_account.id }] }) + end + end + + it_behaves_like 'a deleted branch' + + context 'when service account is not in bypass_settings' do + let(:other_service_account) { create(:user, :service_account) } + + include_context 'with approval policy' do + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { service_accounts: [{ id: other_service_account.id }] }) + end + end + + it 'does not allow delete', :aggregate_failures do + result = execute_service + + expect(result.status).to eq :error + expect(result.message).to eq 'Deleting protected branches is blocked by security policies' + expect(result.reason).to eq :forbidden + end + end + end + + context 'with access token bypass' do + let(:project_bot) { create(:user, :project_bot) } + let!(:personal_access_token) { create(:personal_access_token, user: project_bot) } + + subject(:execute_service) { described_class.new(project, project_bot).execute(protected_branch_name) } + + before_all do + project.add_developer(project_bot) + end + + include_context 'with approval policy' do + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] }) + end + end + + it_behaves_like 'a deleted branch' + + context 'when access token is revoked' do + before do + personal_access_token.revoke! + end + + it 'does not allow delete', :aggregate_failures do + result = execute_service + + expect(result.status).to eq :error + expect(result.message).to eq 'Deleting protected branches is blocked by security policies' + expect(result.reason).to eq :forbidden + end + end + end end context 'when there is a push rule matching the branch name' do diff --git a/ee/spec/services/security/security_orchestration_policies/protected_branches_deletion_check_service_spec.rb b/ee/spec/services/security/security_orchestration_policies/protected_branches_deletion_check_service_spec.rb index 5737cf3ab02e354deb2f7ffb5d201d9067a8b23d..bc0603686d2f0cc544f6778980943a9940947baf 100644 --- a/ee/spec/services/security/security_orchestration_policies/protected_branches_deletion_check_service_spec.rb +++ b/ee/spec/services/security/security_orchestration_policies/protected_branches_deletion_check_service_spec.rb @@ -6,7 +6,8 @@ let_it_be(:project) { create(:project, :repository) } let_it_be(:protected_branch) { create(:protected_branch, project: project) } let(:policy_configuration) { create(:security_orchestration_policy_configuration, project: protected_branch.project) } - let(:result) { described_class.new(project: project).execute([protected_branch]) } + let(:current_user) { nil } + let(:result) { described_class.new(project: project, current_user: current_user).execute([protected_branch]) } before_all do project.repository.add_branch(project.creator, protected_branch.name, "HEAD") @@ -74,5 +75,108 @@ end end end + + context 'with service account bypass' do + let(:service_account) { create(:user, :service_account) } + let(:current_user) { service_account } + + include_context 'with approval policy' do + let(:branch_name) { protected_branch.name } + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { service_accounts: [{ id: service_account.id }] }) + end + end + + it "excludes the protected branch when service account is in bypass_settings" do + expect(result).to exclude(protected_branch) + end + + context 'when service account is not in bypass_settings' do + let(:other_service_account) { create(:user, :service_account) } + + include_context 'with approval policy' do + let(:branch_name) { protected_branch.name } + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { service_accounts: [{ id: other_service_account.id }] }) + end + end + + it "includes the protected branch" do + expect(result).to include(protected_branch) + end + end + + context 'when current_user is nil' do + let(:current_user) { nil } + + include_context 'with approval policy' do + let(:branch_name) { protected_branch.name } + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { service_accounts: [{ id: service_account.id }] }) + end + end + + it "includes the protected branch" do + expect(result).to include(protected_branch) + end + end + end + + context 'with access token bypass' do + let(:project_bot) { create(:user, :project_bot) } + let!(:personal_access_token) { create(:personal_access_token, user: project_bot) } + let(:current_user) { project_bot } + + include_context 'with approval policy' do + let(:branch_name) { protected_branch.name } + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] }) + end + end + + it "excludes the protected branch when access token is in bypass_settings" do + expect(result).to exclude(protected_branch) + end + + context 'when access token is revoked' do + before do + personal_access_token.revoke! + end + + it "includes the protected branch" do + expect(result).to include(protected_branch) + end + end + + context 'when access token is not in bypass_settings' do + let(:other_token) { create(:personal_access_token) } + + include_context 'with approval policy' do + let(:branch_name) { protected_branch.name } + let(:approval_policy) do + build(:approval_policy, + branches: [branch_name], + approval_settings: { block_branch_modification: true }, + bypass_settings: { access_tokens: [{ id: other_token.id }] }) + end + end + + it "includes the protected branch" do + expect(result).to include(protected_branch) + end + end + end end end