From f7e9ec656b445ce28c58e4cabe627229ee74c53e Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 12 Jun 2025 00:05:14 +0200 Subject: [PATCH 01/23] Add bypass_settings to MR approval policy This change adds bypass_settings to security policy schema and updates the branch exceptions to be configured at the policy level. EE: true Changelog: added --- .../security_orchestration_policy.json | 341 +++++++++--------- 1 file changed, 170 insertions(+), 171 deletions(-) diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index 488cc29ad554bc..715c5b717b4f4c 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -1692,201 +1692,200 @@ } } } - } - }, - "additionalProperties": false + }, + "additionalProperties": false + } } - } - }, - "additionalProperties": false, - "$defs": { - "policy_scope": { - "type": "object", - "properties": { - "compliance_frameworks": { - "description": "Specifies for which compliance frameworks this policy should be applied to.", - "type": "array", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer" + }, + "additionalProperties": false, + "$defs": { + "policy_scope": { + "type": "object", + "properties": { + "compliance_frameworks": { + "description": "Specifies for which compliance frameworks this policy should be applied to.", + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer" + } } } - } - }, - "projects": { - "type": "object", - "description": "Specifies for which projects this policy should be applied to.", - "properties": { - "including": { - "type": "array", - "description": "Specifies projects where this policy should be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the project." + }, + "projects": { + "type": "object", + "description": "Specifies for which projects this policy should be applied to.", + "properties": { + "including": { + "type": "array", + "description": "Specifies projects where this policy should be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the project." + } } } - } - }, - "excluding": { - "type": "array", - "description": "Specifies projects where this policy should not be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the project." + }, + "excluding": { + "type": "array", + "description": "Specifies projects where this policy should not be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the project." + } } } } } - } - }, - "groups": { - "type": "object", - "description": "Specifies for which groups this policy should be applied to.", - "properties": { - "including": { - "type": "array", - "description": "Specifies groups where this policy should be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the group." + }, + "groups": { + "type": "object", + "description": "Specifies for which groups this policy should be applied to.", + "properties": { + "including": { + "type": "array", + "description": "Specifies groups where this policy should be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the group." + } } } - } - }, - "excluding": { - "type": "array", - "description": "Specifies groups where this policy should not be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the group." + }, + "excluding": { + "type": "array", + "description": "Specifies groups where this policy should not be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the group." + } } } } } } } - } - }, - "pipeline_execution_content": { - "description": "Specifies the content of custom configuration.", - "type": "object", - "properties": { - "include": { - "type": "array", - "maxItems": 1, - "minItems": 1, - "items": { - "type": "object", - "properties": { - "project": { - "type": "string" - }, - "file": { - "type": "string" - }, - "ref": { - "type": "string" - } - }, - "required": [ - "project", - "file" - ], - "additionalProperties": false - } - } }, - "required": [ - "include" - ], - "additionalProperties": false - }, - "metadata": { - "description": "Specifies custom annotations in a form of key-value pairs.", - "type": "object", - "patternProperties": { - "^[a-zA-Z$][\\w$]*$": { - "type": [ - "string", - "number", - "boolean" - ] - } - }, - "additionalProperties": false - }, - "licenses_with_package_exclusions": { - "type": "array", - "minItems": 1, - "maxItems": 1000, - "uniqueItems": true, - "additionalItems": false, - "items": { + "pipeline_execution_content": { + "description": "Specifies the content of custom configuration.", "type": "object", "properties": { - "name": { - "type": "string", - "minLength": 1, - "maxLength": 255 - }, - "packages": { - "type": "object", - "additionalProperties": false, - "properties": { - "excluding": { - "type": "object", - "additionalProperties": false, - "properties": { - "purls": { - "type": "array", - "minItems": 1, - "maxItems": 1000, - "additionalItems": false, - "items": { - "minLength": 1, - "maxLength": 1024, - "type": "string", - "format": "uri" - } - } + "include": { + "type": "array", + "maxItems": 1, + "minItems": 1, + "items": { + "type": "object", + "properties": { + "project": { + "type": "string" }, - "required": [ - "purls" - ] - } - }, - "required": [ - "excluding" - ] + "file": { + "type": "string" + }, + "ref": { + "type": "string" + } + }, + "required": [ + "project", + "file" + ], + "additionalProperties": false + } } }, "required": [ - "name" - ] - } - }, - "schedule_branches": { - "type": "array", - "description": "List of branches to schedule pipelines for. Maximum five branches per schedule. The policy only schedules pipelines for branches that exist in the project.", - "maxItems": 5, - "uniqueItems": true, - "items": { - "type": "string" + "include" + ], + "additionalProperties": false + }, + "metadata": { + "description": "Specifies custom annotations in a form of key-value pairs.", + "type": "object", + "patternProperties": { + "^[a-zA-Z$][\\w$]*$": { + "type": [ + "string", + "number", + "boolean" + ] + } + }, + "additionalProperties": false + }, + "licenses_with_package_exclusions": { + "type": "array", + "minItems": 1, + "maxItems": 1000, + "uniqueItems": true, + "additionalItems": false, + "items": { + "type": "object", + "properties": { + "name": { + "type": "string", + "minLength": 1, + "maxLength": 255 + }, + "packages": { + "type": "object", + "additionalProperties": false, + "properties": { + "excluding": { + "type": "object", + "additionalProperties": false, + "properties": { + "purls": { + "type": "array", + "minItems": 1, + "maxItems": 1000, + "additionalItems": false, + "items": { + "minLength": 1, + "maxLength": 1024, + "type": "string", + "format": "uri" + } + } + }, + "required": [ + "purls" + ] + } + }, + "required": [ + "excluding" + ] + } + }, + "required": [ + "name" + ] + } + }, + "schedule_branches": { + "type": "array", + "description": "List of branches to schedule pipelines for. Maximum five branches per schedule. The policy only schedules pipelines for branches that exist in the project.", + "maxItems": 5, + "uniqueItems": true, + "items": { + "type": "string" + } } } } -} -- GitLab From ebd2cc9838f79f7a96c76902ad73e946c3668b7e Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 12 Jun 2025 00:08:42 +0200 Subject: [PATCH 02/23] Fix json schemas --- .../validators/json_schemas/security_orchestration_policy.json | 1 + 1 file changed, 1 insertion(+) diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index 715c5b717b4f4c..6ae9ccf510839f 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -1889,3 +1889,4 @@ } } } +} -- GitLab From 328786d60b8a8e27cb31f023afa07bdde1eb1da6 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 18 Jun 2025 21:38:04 +0200 Subject: [PATCH 03/23] Update policy schema JSON --- .../security_orchestration_policy.json | 340 +++++++++--------- 1 file changed, 170 insertions(+), 170 deletions(-) diff --git a/ee/app/validators/json_schemas/security_orchestration_policy.json b/ee/app/validators/json_schemas/security_orchestration_policy.json index 6ae9ccf510839f..488cc29ad554bc 100644 --- a/ee/app/validators/json_schemas/security_orchestration_policy.json +++ b/ee/app/validators/json_schemas/security_orchestration_policy.json @@ -1692,200 +1692,200 @@ } } } - }, - "additionalProperties": false - } + } + }, + "additionalProperties": false } - }, - "additionalProperties": false, - "$defs": { - "policy_scope": { - "type": "object", - "properties": { - "compliance_frameworks": { - "description": "Specifies for which compliance frameworks this policy should be applied to.", - "type": "array", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer" - } - } - } - }, - "projects": { + } + }, + "additionalProperties": false, + "$defs": { + "policy_scope": { + "type": "object", + "properties": { + "compliance_frameworks": { + "description": "Specifies for which compliance frameworks this policy should be applied to.", + "type": "array", + "items": { "type": "object", - "description": "Specifies for which projects this policy should be applied to.", "properties": { - "including": { - "type": "array", - "description": "Specifies projects where this policy should be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the project." - } + "id": { + "type": "integer" + } + } + } + }, + "projects": { + "type": "object", + "description": "Specifies for which projects this policy should be applied to.", + "properties": { + "including": { + "type": "array", + "description": "Specifies projects where this policy should be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the project." } } - }, - "excluding": { - "type": "array", - "description": "Specifies projects where this policy should not be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the project." - } + } + }, + "excluding": { + "type": "array", + "description": "Specifies projects where this policy should not be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the project." } } } } - }, - "groups": { - "type": "object", - "description": "Specifies for which groups this policy should be applied to.", - "properties": { - "including": { - "type": "array", - "description": "Specifies groups where this policy should be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the group." - } + } + }, + "groups": { + "type": "object", + "description": "Specifies for which groups this policy should be applied to.", + "properties": { + "including": { + "type": "array", + "description": "Specifies groups where this policy should be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the group." } } - }, - "excluding": { - "type": "array", - "description": "Specifies groups where this policy should not be applied to.", - "items": { - "type": "object", - "properties": { - "id": { - "type": "integer", - "description": "Specifies the ID of the group." - } + } + }, + "excluding": { + "type": "array", + "description": "Specifies groups where this policy should not be applied to.", + "items": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "description": "Specifies the ID of the group." } } } } } } - }, - "pipeline_execution_content": { - "description": "Specifies the content of custom configuration.", - "type": "object", - "properties": { - "include": { - "type": "array", - "maxItems": 1, - "minItems": 1, - "items": { - "type": "object", - "properties": { - "project": { - "type": "string" - }, - "file": { - "type": "string" - }, - "ref": { - "type": "string" - } + } + }, + "pipeline_execution_content": { + "description": "Specifies the content of custom configuration.", + "type": "object", + "properties": { + "include": { + "type": "array", + "maxItems": 1, + "minItems": 1, + "items": { + "type": "object", + "properties": { + "project": { + "type": "string" }, - "required": [ - "project", - "file" - ], - "additionalProperties": false - } + "file": { + "type": "string" + }, + "ref": { + "type": "string" + } + }, + "required": [ + "project", + "file" + ], + "additionalProperties": false } - }, - "required": [ - "include" - ], - "additionalProperties": false + } + }, + "required": [ + "include" + ], + "additionalProperties": false + }, + "metadata": { + "description": "Specifies custom annotations in a form of key-value pairs.", + "type": "object", + "patternProperties": { + "^[a-zA-Z$][\\w$]*$": { + "type": [ + "string", + "number", + "boolean" + ] + } }, - "metadata": { - "description": "Specifies custom annotations in a form of key-value pairs.", + "additionalProperties": false + }, + "licenses_with_package_exclusions": { + "type": "array", + "minItems": 1, + "maxItems": 1000, + "uniqueItems": true, + "additionalItems": false, + "items": { "type": "object", - "patternProperties": { - "^[a-zA-Z$][\\w$]*$": { - "type": [ - "string", - "number", - "boolean" + "properties": { + "name": { + "type": "string", + "minLength": 1, + "maxLength": 255 + }, + "packages": { + "type": "object", + "additionalProperties": false, + "properties": { + "excluding": { + "type": "object", + "additionalProperties": false, + "properties": { + "purls": { + "type": "array", + "minItems": 1, + "maxItems": 1000, + "additionalItems": false, + "items": { + "minLength": 1, + "maxLength": 1024, + "type": "string", + "format": "uri" + } + } + }, + "required": [ + "purls" + ] + } + }, + "required": [ + "excluding" ] } }, - "additionalProperties": false - }, - "licenses_with_package_exclusions": { - "type": "array", - "minItems": 1, - "maxItems": 1000, - "uniqueItems": true, - "additionalItems": false, - "items": { - "type": "object", - "properties": { - "name": { - "type": "string", - "minLength": 1, - "maxLength": 255 - }, - "packages": { - "type": "object", - "additionalProperties": false, - "properties": { - "excluding": { - "type": "object", - "additionalProperties": false, - "properties": { - "purls": { - "type": "array", - "minItems": 1, - "maxItems": 1000, - "additionalItems": false, - "items": { - "minLength": 1, - "maxLength": 1024, - "type": "string", - "format": "uri" - } - } - }, - "required": [ - "purls" - ] - } - }, - "required": [ - "excluding" - ] - } - }, - "required": [ - "name" - ] - } - }, - "schedule_branches": { - "type": "array", - "description": "List of branches to schedule pipelines for. Maximum five branches per schedule. The policy only schedules pipelines for branches that exist in the project.", - "maxItems": 5, - "uniqueItems": true, - "items": { - "type": "string" - } + "required": [ + "name" + ] + } + }, + "schedule_branches": { + "type": "array", + "description": "List of branches to schedule pipelines for. Maximum five branches per schedule. The policy only schedules pipelines for branches that exist in the project.", + "maxItems": 5, + "uniqueItems": true, + "items": { + "type": "string" } } } -- GitLab From ee901aff5f12ad850ef3c60e88a64226be5b26d5 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Mon, 30 Jun 2025 21:46:52 +0200 Subject: [PATCH 04/23] Add access_tokens to orchestration_policy_data --- ee/app/helpers/ee/security_orchestration_helper.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/ee/app/helpers/ee/security_orchestration_helper.rb b/ee/app/helpers/ee/security_orchestration_helper.rb index 0c4167c39768b8..c03965b91e478d 100644 --- a/ee/app/helpers/ee/security_orchestration_helper.rb +++ b/ee/app/helpers/ee/security_orchestration_helper.rb @@ -209,4 +209,18 @@ def access_tokens_for_container(container) def software_licenses ::Gitlab::SPDX::Catalogue.latest_active_license_names end + + def access_tokens_for_container(container) + bot_users = if container.is_a?(::Project) + container.bots + else + User.by_bot_namespace_ids(container.self_and_descendants(skope: Namespace).as_ids) + end + + PersonalAccessTokensFinder + .new({ users: bot_users, impersonation: false, include_missing: false }) + .execute + .select(:id, :name) + .map { |t| { id: t.id, name: t.name } } + end end -- GitLab From 52e5590f852c3bae3a51c5b68df9b46c9871950f Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Tue, 1 Jul 2025 10:59:54 +0200 Subject: [PATCH 05/23] Handle access_token and service_account bypass from security policy This change adds a new check to pushes to branches that checks if the access_token or service account configured in security policy bypass_settings are allowed to push. EE: true Changelog: added --- ee/app/models/security/policy.rb | 4 + .../ee/gitlab/checks/security/policy_check.rb | 7 ++ .../policy_bypass_checker.rb | 73 +++++++++++++++++++ .../push_bypass_checker.rb | 37 ++++++++++ 4 files changed, 121 insertions(+) create mode 100644 ee/lib/security/scan_result_policies/policy_bypass_checker.rb create mode 100644 ee/lib/security/scan_result_policies/push_bypass_checker.rb diff --git a/ee/app/models/security/policy.rb b/ee/app/models/security/policy.rb index fc61c728f94b94..6de96a246ae64e 100644 --- a/ee/app/models/security/policy.rb +++ b/ee/app/models/security/policy.rb @@ -74,6 +74,10 @@ class Policy < ApplicationRecord where("content->'actions' @> ?", [{ role_approvers: [custom_role_id] }].to_json) end + scope :with_bypass_settings, -> do + where("content->'bypass_settings' IS NOT NULL").where("content->'bypass_settings' <> '{}'::jsonb") + end + delegate :namespace?, :namespace, :project?, :project, to: :security_orchestration_policy_configuration def self.checksum(policy_hash) diff --git a/ee/lib/ee/gitlab/checks/security/policy_check.rb b/ee/lib/ee/gitlab/checks/security/policy_check.rb index 4189428d9045e3..18fb739b92c296 100644 --- a/ee/lib/ee/gitlab/checks/security/policy_check.rb +++ b/ee/lib/ee/gitlab/checks/security/policy_check.rb @@ -14,6 +14,7 @@ def validate! logger.log_timed(LOG_MESSAGE) do break unless branch_name_affected_by_policy? + break if policies_bypass_applied? if force_push? raise ::Gitlab::GitAccess::ForbiddenError, FORCE_PUSH_ERROR_MESSAGE @@ -32,6 +33,12 @@ def branch_name_affected_by_policy? branch_name.in? affected_branches end + def policy_bypass_applied? + return false if Feature.disabled?(:security_policies_bypass_options, project) + + Security::ScanResultPolicies::PushBypassChecker.new(project, user_access).check_bypass! + end + def force_push? ::Gitlab::Checks::ForcePush.force_push?(project, oldrev, newrev) end diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb new file mode 100644 index 00000000000000..82d34bbc5eb13d --- /dev/null +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class PolicyBypassChecker + include Gitlab::Utils::StrongMemoize + + def initialize(security_policy:, project:, user_access:, branch_name:) + @security_policy = security_policy + @project = project + @user_access = user_access + @branch_name = branch_name + end + + def check_bypass! + return false if bypass_settings.blank? + + bypass_with_access_token? || bypass_with_service_account? + end + + def bypass_with_access_token? + return false if bypass_settings[:access_tokens].blank? + + token_info = ::Current.token_info + return false unless token_info && token_info[:token_type] == 'PersonalAccessToken' + + token_id = token_info[:token_id] + token_ids = bypass_settings[:access_tokens].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash + + if token_ids.include?(token_id) + log_bypass_audit!(:access_token, token_id) + true + else + false + end + end + + def bypass_with_service_account? + return false if bypass_settings[:service_accounts].blank? + + user = user_access.respond_to?(:user) ? user_access.user : nil + return false unless user && user.respond_to?(:service_account?) && user.service_account? + + allowed_ids = bypass_settings[:service_accounts].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash + if allowed_ids.include?(user.id) + log_bypass_audit!(:service_account, user.id) + true + else + false + end + end + + private + + attr_reader :security_policy, :project, :user_access, :branch_name + + def bypass_settings + security_policy.policy_content[:bypass_settings] + end + strong_memoize_attr :bypass_settings + + def log_bypass_audit!(type, id) + Gitlab::Audit::Auditor.audit( + name: "security_policy_bypass_#{type}", + author: user_access.respond_to?(:user) ? user_access.user : nil, + scope: project, + target: security_policy, + message: "Branch push bypassed by security policy #{security_policy.name} with #{type} ID: #{id}" + ) + end + end + end +end diff --git a/ee/lib/security/scan_result_policies/push_bypass_checker.rb b/ee/lib/security/scan_result_policies/push_bypass_checker.rb new file mode 100644 index 00000000000000..a9ef9286a87e3d --- /dev/null +++ b/ee/lib/security/scan_result_policies/push_bypass_checker.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class PushBypassChecker + include Gitlab::Utils::StrongMemoize + + def initialize(project:, user_access:, branch_name:) + @project = project + @user_access = user_access + @branch_name = branch_name + end + + def check_bypass! + return unless project.licensed_feature_available?(:security_orchestration_policies) + + policies = project.security_policies.with_bypass_settings + return if policies.empty? + + policies.all? { |policy| bypass_allowed?(policy) } + end + + private + + attr_reader :project, :user_access, :branch_name + + def bypass_allowed?(policy) + Security::ScanResultPolicies::PolicyBypassChecker.new( + security_policy: policy, + project: project, + user_access: user_access, + branch_name: branch_name + ).check_bypass! + end + end + end +end -- GitLab From 235bc4f16da5c88b961cd55c0e2c4f15f60ef382 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Mon, 7 Jul 2025 13:41:08 +0200 Subject: [PATCH 06/23] Add audit event definition and add spec --- ...curity_policy_access_token_push_bypass.yml | 11 +++ ...ity_policy_service_account_push_bypass.yml | 11 +++ .../ee/gitlab/checks/security/policy_check.rb | 8 +- .../policy_bypass_checker.rb | 5 +- .../checks/security/policy_check_spec.rb | 28 ++++++ .../policy_bypass_checker_spec.rb | 87 +++++++++++++++++++ .../push_bypass_checker_spec.rb | 71 +++++++++++++++ 7 files changed, 216 insertions(+), 5 deletions(-) create mode 100644 config/audit_events/types/security_policy_access_token_push_bypass.yml create mode 100644 config/audit_events/types/security_policy_service_account_push_bypass.yml create mode 100644 ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb create mode 100644 ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb diff --git a/config/audit_events/types/security_policy_access_token_push_bypass.yml b/config/audit_events/types/security_policy_access_token_push_bypass.yml new file mode 100644 index 00000000000000..c0c8e4662379f6 --- /dev/null +++ b/config/audit_events/types/security_policy_access_token_push_bypass.yml @@ -0,0 +1,11 @@ +--- +name: security_policy_access_token_push_bypass +description: Branch push that is blocked by a security policy is bypassed for configured + access token +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549644 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249 +feature_category: security_policy_management +milestone: '18.2' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/config/audit_events/types/security_policy_service_account_push_bypass.yml b/config/audit_events/types/security_policy_service_account_push_bypass.yml new file mode 100644 index 00000000000000..d497d696fa906c --- /dev/null +++ b/config/audit_events/types/security_policy_service_account_push_bypass.yml @@ -0,0 +1,11 @@ +--- +name: security_policy_service_account_push_bypass +description: Branch push that is blocked by a security policy is bypassed for configured + service account +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549644 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249 +feature_category: security_policy_management +milestone: '18.2' +saved_to_database: true +scope: [Project] +streamed: true diff --git a/ee/lib/ee/gitlab/checks/security/policy_check.rb b/ee/lib/ee/gitlab/checks/security/policy_check.rb index 18fb739b92c296..55745c9ae34367 100644 --- a/ee/lib/ee/gitlab/checks/security/policy_check.rb +++ b/ee/lib/ee/gitlab/checks/security/policy_check.rb @@ -33,10 +33,12 @@ def branch_name_affected_by_policy? branch_name.in? affected_branches end - def policy_bypass_applied? - return false if Feature.disabled?(:security_policies_bypass_options, project) + def policies_bypass_applied? + return false if ::Feature.disabled?(:security_policies_bypass_options, project) - Security::ScanResultPolicies::PushBypassChecker.new(project, user_access).check_bypass! + ::Security::ScanResultPolicies::PushBypassChecker.new( + project: project, user_access: user_access, branch_name: branch_name + ).check_bypass! end def force_push? diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index 82d34bbc5eb13d..c85e81a2ad6361 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -61,11 +61,12 @@ def bypass_settings def log_bypass_audit!(type, id) Gitlab::Audit::Auditor.audit( - name: "security_policy_bypass_#{type}", + name: "security_policy_#{type}_push_bypass", author: user_access.respond_to?(:user) ? user_access.user : nil, scope: project, target: security_policy, - message: "Branch push bypassed by security policy #{security_policy.name} with #{type} ID: #{id}" + message: + "Branch push blocked by security policy #{security_policy.name} is bypassed for #{type} with ID: #{id}" ) end end diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index a8363c351081a8..6790cbf5b49e55 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -101,5 +101,33 @@ expect { policy_check! }.not_to raise_error end end + + context 'when policies_bypass_applied? allows bypass with access token' do + let(:user) { create(:user) } + let!(:personal_access_token) { create(:personal_access_token, user: user) } + + before do + # Simulate the token being used + ::Current.token_info = { token_id: personal_access_token.id, token_type: 'PersonalAccessToken' } + + create(:security_policy, + :approval_policy, + security_orchestration_policy_configuration: policy_configuration, + linked_projects: [project], + content: { + actions: [], + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } + } + ) + end + + after do + ::Current.reset + end + + it 'does not raise' do + expect { policy_check! }.not_to raise_error + end + end end end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb new file mode 100644 index 00000000000000..cfbece50d99ac7 --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::PolicyBypassChecker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project) } + let_it_be(:branch_name) { 'main' } + let_it_be(:user) { create(:user) } + let_it_be(:service_account) { create(:service_account) } + let_it_be(:default_user_access) { Gitlab::UserAccess.new(user, container: project) } + let_it_be(:service_account_access) { Gitlab::UserAccess.new(service_account, container: project) } + let_it_be_with_refind(:security_policy) do + create(:security_policy, + :approval_policy, + security_orchestration_policy_configuration: policy_configuration, + linked_projects: [project], + content: { actions: [], bypass_settings: {} } + ) + end + + let_it_be(:user_access) { default_user_access } + + subject(:checker) do + described_class.new(security_policy: security_policy, project: project, user_access: user_access, + branch_name: branch_name) + end + + describe '#check_bypass!' do + context 'when bypass_settings is blank' do + before do + security_policy.update!(content: { actions: [] }) + end + + it 'returns false' do + expect(checker.check_bypass!).to be false + end + end + + context 'when bypass_settings has access_tokens' do + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + before do + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } }) + ::Current.token_info = { token_id: personal_access_token.id, token_type: 'PersonalAccessToken' } + end + + after do + ::Current.reset + end + + it 'returns true if the access token is allowed to bypass' do + expect(checker.check_bypass!).to be true + end + + it 'returns false if the access token is not allowed to bypass' do + ::Current.token_info = { token_id: -1, token_type: 'PersonalAccessToken' } + expect(checker.check_bypass!).to be false + end + end + + context 'when bypass_settings has service_accounts' do + before do + security_policy.update!(content: { bypass_settings: { service_accounts: [{ id: service_account.id }] } }) + end + + context 'when user_access is the allowed service account' do + let_it_be(:user_access) { service_account_access } + + it 'returns true if the service account is allowed to bypass' do + expect(checker.check_bypass!).to be true + end + end + + context 'when user_access is a different service account' do + let_it_be(:user_access) do + other_service_account = create(:service_account) + Gitlab::UserAccess.new(other_service_account, container: project) + end + + it 'returns false if the service account is not allowed to bypass' do + expect(checker.check_bypass!).to be false + end + end + end + end +end diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb new file mode 100644 index 00000000000000..7a8021fb997e6b --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::PushBypassChecker, feature_category: :security_policy_management do + let_it_be(:project) { create(:project) } + let_it_be(:policy_project) { create(:project) } + let_it_be(:policy_configuration) do + create(:security_orchestration_policy_configuration, project: project, + security_policy_management_project: policy_project) + end + + let_it_be(:branch_name) { 'main' } + let_it_be(:user) { create(:user) } + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + let_it_be(:checker) { described_class.new(project: project, user_access: user_access, branch_name: branch_name) } + + describe '#check_bypass!' do + context 'when the feature is not available' do + before do + stub_licensed_features(security_orchestration_policies: false) + end + + it 'returns nil' do + expect(checker.check_bypass!).to be_nil + end + end + + context 'when the feature is available' do + before do + stub_licensed_features(security_orchestration_policies: true) + end + + context 'when there are no policies with bypass settings' do + it 'returns nil' do + expect(checker.check_bypass!).to be_nil + end + end + + context 'when there is a policy with bypass settings for access token' do + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + before do + ::Current.token_info = { token_id: personal_access_token.id, token_type: 'PersonalAccessToken' } + create(:security_policy, + :approval_policy, + security_orchestration_policy_configuration: policy_configuration, + linked_projects: [project], + content: { + actions: [], + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } + } + ) + end + + after do + ::Current.reset + end + + it 'returns true if the access token is allowed to bypass' do + expect(checker.check_bypass!).to be true + end + + it 'returns false if the access token is not allowed to bypass' do + ::Current.token_info = { token_id: -1, token_type: 'PersonalAccessToken' } + expect(checker.check_bypass!).to be false + end + end + end + end +end -- GitLab From f444916fe7eabd9a4f46cd71c29e835f89ba9ed1 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Mon, 7 Jul 2025 13:45:29 +0200 Subject: [PATCH 07/23] Add docs for audit events --- doc/user/compliance/audit_event_types.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/user/compliance/audit_event_types.md b/doc/user/compliance/audit_event_types.md index cfa97ca3e2d47f..883bd0be7119df 100644 --- a/doc/user/compliance/audit_event_types.md +++ b/doc/user/compliance/audit_event_types.md @@ -546,6 +546,8 @@ Audit event types belong to the following product categories. | [`policy_violations_detected`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violation is detected in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/549811) | Project | | [`policy_violations_resolved`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/193482) | Security policy violations are resolved in the merge request | {{< icon name="dotted-circle" >}} No | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549812) | Project | | [`policy_yaml_invalidated`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196721) | The policy YAML is invalidated in security policy project | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/work_items/550892) | Project | +| [`security_policy_access_token_push_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249) | Branch push that is blocked by a security policy is bypassed for configured access token | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549644) | Project | +| [`security_policy_service_account_push_bypass`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249) | Branch push that is blocked by a security policy is bypassed for configured service account | {{< icon name="check-circle" >}} Yes | GitLab [18.2](https://gitlab.com/gitlab-org/gitlab/-/issues/549644) | Project | ### Security testing configuration -- GitLab From 0615137ddd03bcd28de21b55367a83e4f3639a40 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Mon, 7 Jul 2025 16:39:16 +0200 Subject: [PATCH 08/23] Add spec for scope --- ee/spec/models/security/policy_spec.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/ee/spec/models/security/policy_spec.rb b/ee/spec/models/security/policy_spec.rb index 0f5ab394f68d70..1d05ddc0a70730 100644 --- a/ee/spec/models/security/policy_spec.rb +++ b/ee/spec/models/security/policy_spec.rb @@ -1259,4 +1259,21 @@ }) end end + + describe '.with_bypass_settings' do + let_it_be(:policy_with_bypass) do + create(:security_policy, content: { bypass_settings: { access_tokens: [{ id: 1 }] } }) + end + + let_it_be(:policy_without_bypass) do + create(:security_policy, :require_approval) + end + + let_it_be(:policy_with_empty_bypass) { create(:security_policy, content: { bypass_settings: {} }) } + + it 'returns only policies with non-empty bypass_settings' do + result = described_class.with_bypass_settings + expect(result).to contain_exactly(policy_with_bypass) + end + end end -- GitLab From 9c632cc61b493e36cf800f0f0e33b42806848f8b Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Mon, 7 Jul 2025 22:56:05 +0200 Subject: [PATCH 09/23] Refactor access token check and update spec --- .../policy_bypass_checker.rb | 22 ++++++------- .../policy_bypass_checker_spec.rb | 32 ++++++++++++++----- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index c85e81a2ad6361..d1788d7fcd9158 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -21,14 +21,14 @@ def check_bypass! def bypass_with_access_token? return false if bypass_settings[:access_tokens].blank? - token_info = ::Current.token_info - return false unless token_info && token_info[:token_type] == 'PersonalAccessToken' + user = user_access.user + return false unless user - token_id = token_info[:token_id] - token_ids = bypass_settings[:access_tokens].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash + policy_token_ids = bypass_settings[:access_tokens].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash + user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key - if token_ids.include?(token_id) - log_bypass_audit!(:access_token, token_id) + if user_token_ids.any? + log_bypass_audit!(:access_token, user_token_ids) true else false @@ -38,11 +38,11 @@ def bypass_with_access_token? def bypass_with_service_account? return false if bypass_settings[:service_accounts].blank? - user = user_access.respond_to?(:user) ? user_access.user : nil - return false unless user && user.respond_to?(:service_account?) && user.service_account? + user = user_access.user + return false unless user && user.service_account? - allowed_ids = bypass_settings[:service_accounts].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash - if allowed_ids.include?(user.id) + policy_service_account_ids = bypass_settings[:service_accounts].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash + if policy_service_account_ids.include?(user.id) log_bypass_audit!(:service_account, user.id) true else @@ -66,7 +66,7 @@ def log_bypass_audit!(type, id) scope: project, target: security_policy, message: - "Branch push blocked by security policy #{security_policy.name} is bypassed for #{type} with ID: #{id}" + "Blocked branch push is bypassed by security policy '#{security_policy.name}' for #{type} with ID: #{id}" ) end end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index cfbece50d99ac7..f15dddffa639a3 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -42,20 +42,36 @@ before do security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } }) - ::Current.token_info = { token_id: personal_access_token.id, token_type: 'PersonalAccessToken' } end - after do - ::Current.reset + context 'when the access token is inactive' do + before do + personal_access_token.update!(expires_at: 1.day.ago) + end + + it 'returns false' do + expect(checker.check_bypass!).to be false + end end - it 'returns true if the access token is allowed to bypass' do - expect(checker.check_bypass!).to be true + context 'when the access token is active' do + before do + personal_access_token.update!(expires_at: nil) + end + + it 'returns true' do + expect(checker.check_bypass!).to be true + end end - it 'returns false if the access token is not allowed to bypass' do - ::Current.token_info = { token_id: -1, token_type: 'PersonalAccessToken' } - expect(checker.check_bypass!).to be false + context 'when the access token is not allowed to bypass' do + before do + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: -1 }] } }) + end + + it 'returns false' do + expect(checker.check_bypass!).to be false + end end end -- GitLab From 52d6680b8a2437d40f8f8205f4d521033a7d6ed5 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Tue, 8 Jul 2025 15:53:15 +0200 Subject: [PATCH 10/23] Fix rubocop errors --- ee/app/helpers/ee/security_orchestration_helper.rb | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/ee/app/helpers/ee/security_orchestration_helper.rb b/ee/app/helpers/ee/security_orchestration_helper.rb index c03965b91e478d..0c4167c39768b8 100644 --- a/ee/app/helpers/ee/security_orchestration_helper.rb +++ b/ee/app/helpers/ee/security_orchestration_helper.rb @@ -209,18 +209,4 @@ def access_tokens_for_container(container) def software_licenses ::Gitlab::SPDX::Catalogue.latest_active_license_names end - - def access_tokens_for_container(container) - bot_users = if container.is_a?(::Project) - container.bots - else - User.by_bot_namespace_ids(container.self_and_descendants(skope: Namespace).as_ids) - end - - PersonalAccessTokensFinder - .new({ users: bot_users, impersonation: false, include_missing: false }) - .execute - .select(:id, :name) - .map { |t| { id: t.id, name: t.name } } - end end -- GitLab From 4c3ac63faf51e3ee73442cea7bb323da849258fe Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Tue, 8 Jul 2025 17:48:25 +0200 Subject: [PATCH 11/23] Fix failing spec --- .../policy_bypass_checker_spec.rb | 3 +- .../push_bypass_checker_spec.rb | 39 +++++++------------ 2 files changed, 16 insertions(+), 26 deletions(-) diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index f15dddffa639a3..bb18592d37ab32 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -66,7 +66,8 @@ context 'when the access token is not allowed to bypass' do before do - security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: -1 }] } }) + another_access_token = create(:personal_access_token) + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: another_access_token.id }] } }) end it 'returns false' do diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb index 7a8021fb997e6b..55544a4c6876fb 100644 --- a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -4,11 +4,6 @@ RSpec.describe Security::ScanResultPolicies::PushBypassChecker, feature_category: :security_policy_management do let_it_be(:project) { create(:project) } - let_it_be(:policy_project) { create(:project) } - let_it_be(:policy_configuration) do - create(:security_orchestration_policy_configuration, project: project, - security_policy_management_project: policy_project) - end let_it_be(:branch_name) { 'main' } let_it_be(:user) { create(:user) } @@ -39,31 +34,25 @@ context 'when there is a policy with bypass settings for access token' do let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - - before do - ::Current.token_info = { token_id: personal_access_token.id, token_type: 'PersonalAccessToken' } - create(:security_policy, - :approval_policy, - security_orchestration_policy_configuration: policy_configuration, - linked_projects: [project], - content: { - actions: [], - bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } - } - ) + let_it_be_with_reload(:security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], content: { + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } + }) end - after do - ::Current.reset - end - - it 'returns true if the access token is allowed to bypass' do + it 'returns true' do expect(checker.check_bypass!).to be true end - it 'returns false if the access token is not allowed to bypass' do - ::Current.token_info = { token_id: -1, token_type: 'PersonalAccessToken' } - expect(checker.check_bypass!).to be false + context 'when the access token is not allowed to bypass' do + before do + another_access_token = create(:personal_access_token) + security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: another_access_token.id }] } }) + end + + it 'returns false' do + expect(checker.check_bypass!).to be false + end end end end -- GitLab From c062d0c6ec1db712c234493af960290bc0a70a26 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 9 Jul 2025 12:05:50 +0200 Subject: [PATCH 12/23] Fix failing spec --- .../policy_bypass_checker.rb | 33 ++++++----- .../policy_bypass_checker_spec.rb | 55 ++++++++----------- 2 files changed, 38 insertions(+), 50 deletions(-) diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index d1788d7fcd9158..818b79b357ee16 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -19,35 +19,29 @@ def check_bypass! end def bypass_with_access_token? - return false if bypass_settings[:access_tokens].blank? + policy_token_ids = bypass_settings[:access_tokens]&.pluck(:id) + return false if policy_token_ids.blank? user = user_access.user return false unless user - policy_token_ids = bypass_settings[:access_tokens].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key + return false if user_token_ids.blank? - if user_token_ids.any? - log_bypass_audit!(:access_token, user_token_ids) - true - else - false - end + log_bypass_audit!(:access_token, user_token_ids) + true end def bypass_with_service_account? - return false if bypass_settings[:service_accounts].blank? + policy_service_account_ids = bypass_settings[:service_accounts]&.pluck(:id) + return false if policy_service_account_ids.blank? user = user_access.user return false unless user && user.service_account? + return false unless policy_service_account_ids.include?(user.id) - policy_service_account_ids = bypass_settings[:service_accounts].pluck(:id) # rubocop:disable CodeReuse/ActiveRecord -- pluck used on hash - if policy_service_account_ids.include?(user.id) - log_bypass_audit!(:service_account, user.id) - true - else - false - end + log_bypass_audit!(:service_account, user.id) + true end private @@ -66,7 +60,12 @@ def log_bypass_audit!(type, id) scope: project, target: security_policy, message: - "Blocked branch push is bypassed by security policy '#{security_policy.name}' for #{type} with ID: #{id}" + "Blocked branch push is bypassed by security policy '#{security_policy.name}' for #{type} with ID: #{id}", + additional_details: { + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + } ) end end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index bb18592d37ab32..62375dc268a281 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -4,37 +4,30 @@ RSpec.describe Security::ScanResultPolicies::PolicyBypassChecker, feature_category: :security_policy_management do let_it_be(:project) { create(:project) } - let_it_be(:policy_configuration) { create(:security_orchestration_policy_configuration, project: project) } let_it_be(:branch_name) { 'main' } let_it_be(:user) { create(:user) } let_it_be(:service_account) { create(:service_account) } - let_it_be(:default_user_access) { Gitlab::UserAccess.new(user, container: project) } - let_it_be(:service_account_access) { Gitlab::UserAccess.new(service_account, container: project) } + let_it_be_with_refind(:security_policy) do - create(:security_policy, - :approval_policy, - security_orchestration_policy_configuration: policy_configuration, - linked_projects: [project], - content: { actions: [], bypass_settings: {} } - ) + create(:security_policy, linked_projects: [project], content: { bypass_settings: {} }) end - let_it_be(:user_access) { default_user_access } - - subject(:checker) do - described_class.new(security_policy: security_policy, project: project, user_access: user_access, - branch_name: branch_name) - end + let_it_be(:service_account_access) { Gitlab::UserAccess.new(service_account, container: project) } + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } describe '#check_bypass!' do + subject(:check_bypass!) do + described_class.new( + security_policy: security_policy, project: project, user_access: user_access, branch_name: branch_name + ).check_bypass! + end + context 'when bypass_settings is blank' do before do security_policy.update!(content: { actions: [] }) end - it 'returns false' do - expect(checker.check_bypass!).to be false - end + it { is_expected.to be false } end context 'when bypass_settings has access_tokens' do @@ -49,9 +42,7 @@ personal_access_token.update!(expires_at: 1.day.ago) end - it 'returns false' do - expect(checker.check_bypass!).to be false - end + it { is_expected.to be false } end context 'when the access token is active' do @@ -59,9 +50,7 @@ personal_access_token.update!(expires_at: nil) end - it 'returns true' do - expect(checker.check_bypass!).to be true - end + it { is_expected.to be true } end context 'when the access token is not allowed to bypass' do @@ -70,9 +59,7 @@ security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: another_access_token.id }] } }) end - it 'returns false' do - expect(checker.check_bypass!).to be false - end + it { is_expected.to be false } end end @@ -84,9 +71,7 @@ context 'when user_access is the allowed service account' do let_it_be(:user_access) { service_account_access } - it 'returns true if the service account is allowed to bypass' do - expect(checker.check_bypass!).to be true - end + it { is_expected.to be true } end context 'when user_access is a different service account' do @@ -95,9 +80,13 @@ Gitlab::UserAccess.new(other_service_account, container: project) end - it 'returns false if the service account is not allowed to bypass' do - expect(checker.check_bypass!).to be false - end + it { is_expected.to be false } + end + + context 'when user_access is not a service account' do + let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } + + it { is_expected.to be false } end end end -- GitLab From 6297a77d3b75e496537e376b9f031d67f8a63d1c Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 9 Jul 2025 12:22:28 +0200 Subject: [PATCH 13/23] Validate if the access_token user is project_bot --- .../scan_result_policies/policy_bypass_checker.rb | 2 +- .../policy_bypass_checker_spec.rb | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index 818b79b357ee16..de8f553002966d 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -23,7 +23,7 @@ def bypass_with_access_token? return false if policy_token_ids.blank? user = user_access.user - return false unless user + return false unless user && user.project_bot? user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key return false if user_token_ids.blank? diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index 62375dc268a281..fa96cb77b545c2 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Security::ScanResultPolicies::PolicyBypassChecker, feature_category: :security_policy_management do let_it_be(:project) { create(:project) } let_it_be(:branch_name) { 'main' } - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user, :project_bot) } let_it_be(:service_account) { create(:service_account) } let_it_be_with_refind(:security_policy) do @@ -61,6 +61,15 @@ it { is_expected.to be false } end + + context 'when user_access is not a project bot' do + let_it_be(:user_access) do + normal_user = create(:user) + Gitlab::UserAccess.new(normal_user, container: project) + end + + it { is_expected.to be false } + end end context 'when bypass_settings has service_accounts' do -- GitLab From 92f2e887b965fa7d6ede30fbc970f41771fdd184 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 9 Jul 2025 14:56:19 +0200 Subject: [PATCH 14/23] Add spec for audit logs --- .../policy_bypass_checker_spec.rb | 63 ++++++++++++++++--- 1 file changed, 55 insertions(+), 8 deletions(-) diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index fa96cb77b545c2..e2c827290b8b49 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -22,12 +22,27 @@ ).check_bypass! end + before do + allow(Gitlab::Audit::Auditor).to receive(:audit).and_call_original + end + + shared_examples 'bypass is not allowed and audit log is not created' do + it 'returns false and does not create an audit log' do + result = check_bypass! + + expect(result).to be false + expect(Gitlab::Audit::Auditor).not_to have_received(:audit).with( + hash_including(message: a_string_including("Blocked branch push is bypassed by security policy")) + ) + end + end + context 'when bypass_settings is blank' do before do security_policy.update!(content: { actions: [] }) end - it { is_expected.to be false } + it_behaves_like 'bypass is not allowed and audit log is not created' end context 'when bypass_settings has access_tokens' do @@ -42,7 +57,7 @@ personal_access_token.update!(expires_at: 1.day.ago) end - it { is_expected.to be false } + it_behaves_like 'bypass is not allowed and audit log is not created' end context 'when the access token is active' do @@ -50,7 +65,23 @@ personal_access_token.update!(expires_at: nil) end - it { is_expected.to be true } + it 'returns true and creates an audit log' do + result = check_bypass! + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( + name: 'security_policy_access_token_push_bypass', + author: user, + scope: project, + target: security_policy, + message: a_string_including("Blocked branch push is bypassed by security policy"), + additional_details: hash_including( + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + ) + )) + end end context 'when the access token is not allowed to bypass' do @@ -59,7 +90,7 @@ security_policy.update!(content: { bypass_settings: { access_tokens: [{ id: another_access_token.id }] } }) end - it { is_expected.to be false } + it_behaves_like 'bypass is not allowed and audit log is not created' end context 'when user_access is not a project bot' do @@ -68,7 +99,7 @@ Gitlab::UserAccess.new(normal_user, container: project) end - it { is_expected.to be false } + it_behaves_like 'bypass is not allowed and audit log is not created' end end @@ -80,7 +111,23 @@ context 'when user_access is the allowed service account' do let_it_be(:user_access) { service_account_access } - it { is_expected.to be true } + it 'returns true and creates an audit log' do + result = check_bypass! + + expect(result).to be true + expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( + name: 'security_policy_service_account_push_bypass', + author: service_account, + scope: project, + target: security_policy, + message: a_string_including("Blocked branch push is bypassed by security policy"), + additional_details: hash_including( + security_policy_name: security_policy.name, + security_policy_id: security_policy.id, + branch_name: branch_name + ) + )) + end end context 'when user_access is a different service account' do @@ -89,13 +136,13 @@ Gitlab::UserAccess.new(other_service_account, container: project) end - it { is_expected.to be false } + it_behaves_like 'bypass is not allowed and audit log is not created' end context 'when user_access is not a service account' do let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } - it { is_expected.to be false } + it_behaves_like 'bypass is not allowed and audit log is not created' end end end -- GitLab From 27aa420f48cacf1b31aa1bb98ad811f73626f0af Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 9 Jul 2025 16:16:46 +0200 Subject: [PATCH 15/23] Fix failing spec --- .../gitlab/checks/security/policy_check_spec.rb | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index 6790cbf5b49e55..7ae842584e5e02 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -103,28 +103,17 @@ end context 'when policies_bypass_applied? allows bypass with access token' do - let(:user) { create(:user) } - let!(:personal_access_token) { create(:personal_access_token, user: user) } + let_it_be(:user) { create(:user, :project_bot) } + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } before do - # Simulate the token being used - ::Current.token_info = { token_id: personal_access_token.id, token_type: 'PersonalAccessToken' } - - create(:security_policy, - :approval_policy, - security_orchestration_policy_configuration: policy_configuration, - linked_projects: [project], + create(:security_policy, linked_projects: [project], content: { - actions: [], bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } } ) end - after do - ::Current.reset - end - it 'does not raise' do expect { policy_check! }.not_to raise_error end -- GitLab From 5256b1d4f81d3b3ad55e89206f4a28544780a753 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 9 Jul 2025 16:51:49 +0200 Subject: [PATCH 16/23] Fix failing spec --- .../security/scan_result_policies/push_bypass_checker_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb index 55544a4c6876fb..015bea9487b693 100644 --- a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -6,7 +6,7 @@ let_it_be(:project) { create(:project) } let_it_be(:branch_name) { 'main' } - let_it_be(:user) { create(:user) } + let_it_be(:user) { create(:user, :project_bot) } let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } let_it_be(:checker) { described_class.new(project: project, user_access: user_access, branch_name: branch_name) } -- GitLab From 4a06f6da23dec329bb33778e23c622fd5e768e92 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Wed, 9 Jul 2025 21:05:58 +0200 Subject: [PATCH 17/23] Add spec for multiple policies --- .../push_bypass_checker.rb | 2 +- .../push_bypass_checker_spec.rb | 52 +++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/ee/lib/security/scan_result_policies/push_bypass_checker.rb b/ee/lib/security/scan_result_policies/push_bypass_checker.rb index a9ef9286a87e3d..e4a6a2d6d3c2bd 100644 --- a/ee/lib/security/scan_result_policies/push_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/push_bypass_checker.rb @@ -17,7 +17,7 @@ def check_bypass! policies = project.security_policies.with_bypass_settings return if policies.empty? - policies.all? { |policy| bypass_allowed?(policy) } + policies.any? { |policy| bypass_allowed?(policy) } end private diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb index 015bea9487b693..a0ee3868ea1568 100644 --- a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -55,6 +55,58 @@ end end end + + context 'with multiple security policies' do + let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } + + context 'when multiple policies have bypass_settings' do + let_it_be_with_reload(:security_policy1) do + create(:security_policy, :approval_policy, linked_projects: [project], content: { + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } + }) + end + + let_it_be_with_reload(:security_policy2) do + create(:security_policy, :approval_policy, linked_projects: [project], content: { + bypass_settings: { access_tokens: [{ id: 999_999 }] } # not matching + }) + end + + it 'returns true if any policy allows bypass' do + expect(checker.check_bypass!).to be true + end + end + + context 'when only one policy has bypass_settings' do + let_it_be_with_reload(:security_policy1) do + create(:security_policy, :approval_policy, linked_projects: [project], content: { + bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } + }) + end + + let_it_be_with_reload(:security_policy2) do + create(:security_policy, :approval_policy, linked_projects: [project], content: {}) + end + + it 'returns true if the policy with bypass_settings allows bypass' do + expect(checker.check_bypass!).to be true + end + end + + context 'when multiple policies have no bypass_settings' do + let_it_be_with_reload(:security_policy1) do + create(:security_policy, :approval_policy, linked_projects: [project], content: {}) + end + + let_it_be_with_reload(:security_policy2) do + create(:security_policy, :approval_policy, linked_projects: [project], content: {}) + end + + it 'returns nil' do + expect(checker.check_bypass!).to be_nil + end + end + end end end end -- GitLab From 8aebd7b51cfd4b8ed2a6f2f46938a5c3958707dd Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 10 Jul 2025 10:37:14 +0200 Subject: [PATCH 18/23] Update feature flag name and add spec --- ee/lib/ee/gitlab/checks/security/policy_check.rb | 2 +- .../scan_result_policies/policy_bypass_checker.rb | 2 +- .../lib/gitlab/checks/security/policy_check_spec.rb | 12 ++++++++++++ 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ee/lib/ee/gitlab/checks/security/policy_check.rb b/ee/lib/ee/gitlab/checks/security/policy_check.rb index 55745c9ae34367..903fddd8e35995 100644 --- a/ee/lib/ee/gitlab/checks/security/policy_check.rb +++ b/ee/lib/ee/gitlab/checks/security/policy_check.rb @@ -34,7 +34,7 @@ def branch_name_affected_by_policy? end def policies_bypass_applied? - return false if ::Feature.disabled?(:security_policies_bypass_options, project) + return false if ::Feature.disabled?(:security_policies_bypass_options_tokens_accounts, project) ::Security::ScanResultPolicies::PushBypassChecker.new( project: project, user_access: user_access, branch_name: branch_name diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index de8f553002966d..e4d28eabbea176 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -56,7 +56,7 @@ def bypass_settings def log_bypass_audit!(type, id) Gitlab::Audit::Auditor.audit( name: "security_policy_#{type}_push_bypass", - author: user_access.respond_to?(:user) ? user_access.user : nil, + author: user_access.user, scope: project, target: security_policy, message: diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index 7ae842584e5e02..72c3615cc67267 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -118,5 +118,17 @@ expect { policy_check! }.not_to raise_error end end + + context 'when the security_policies_bypass_options_tokens_accounts feature flag is disabled' do + before do + stub_feature_flags(security_policies_bypass_options_tokens_accounts: false) + end + + it 'does not bypass and raises error' do + expect { policy_check! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, described_class::FORCE_PUSH_ERROR_MESSAGE + ) + end + end end end -- GitLab From e2df0c413dcfaea5c8ed2cbe1b0a027e26a2871c Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 10 Jul 2025 10:51:22 +0200 Subject: [PATCH 19/23] Fix potential SQL injection --- ee/app/models/security/policy.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/models/security/policy.rb b/ee/app/models/security/policy.rb index 6de96a246ae64e..f9ca95fdd67f0c 100644 --- a/ee/app/models/security/policy.rb +++ b/ee/app/models/security/policy.rb @@ -75,7 +75,7 @@ class Policy < ApplicationRecord end scope :with_bypass_settings, -> do - where("content->'bypass_settings' IS NOT NULL").where("content->'bypass_settings' <> '{}'::jsonb") + where("content->'bypass_settings' IS NOT NULL").where("content->'bypass_settings' <> ?", '{}') end delegate :namespace?, :namespace, :project?, :project, to: :security_orchestration_policy_configuration -- GitLab From 0a935777592047a0973a364d8adac52ff0662a28 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 10 Jul 2025 19:09:59 +0200 Subject: [PATCH 20/23] Address review comments --- ee/app/models/security/policy.rb | 5 +++ .../scan_result_policies/bypass_settings.rb | 27 +++++++++++++ .../policy_bypass_checker.rb | 25 ++++-------- .../push_bypass_checker.rb | 2 +- ee/spec/factories/security/policies.rb | 21 ++++++++++ .../checks/security/policy_check_spec.rb | 6 +-- .../bypass_settings_spec.rb | 39 +++++++++++++++++++ .../policy_bypass_checker_spec.rb | 12 +++--- .../push_bypass_checker_spec.rb | 28 ++++++------- ee/spec/models/security/policy_spec.rb | 2 +- 10 files changed, 121 insertions(+), 46 deletions(-) create mode 100644 ee/lib/security/scan_result_policies/bypass_settings.rb create mode 100644 ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb diff --git a/ee/app/models/security/policy.rb b/ee/app/models/security/policy.rb index f9ca95fdd67f0c..e2034f4a46945a 100644 --- a/ee/app/models/security/policy.rb +++ b/ee/app/models/security/policy.rb @@ -318,6 +318,11 @@ def enforced_scans=(scans) metadata['enforced_scans'] = scans end + def bypass_settings + Security::ScanResultPolicies::BypassSettings.new(self) + end + strong_memoize_attr :bypass_settings + private def content_by_type diff --git a/ee/lib/security/scan_result_policies/bypass_settings.rb b/ee/lib/security/scan_result_policies/bypass_settings.rb new file mode 100644 index 00000000000000..4e9af3b3d801be --- /dev/null +++ b/ee/lib/security/scan_result_policies/bypass_settings.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Security + module ScanResultPolicies + class BypassSettings + include Gitlab::Utils::StrongMemoize + + def initialize(security_policy) + @bypass_settings = security_policy.policy_content[:bypass_settings] || {} + end + + def access_token_ids + bypass_settings[:access_tokens]&.pluck(:id) + end + strong_memoize_attr :access_token_ids + + def service_account_ids + bypass_settings[:service_accounts]&.pluck(:id) + end + strong_memoize_attr :service_account_ids + + private + + attr_reader :bypass_settings + end + end +end diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index e4d28eabbea176..22a4fc10b379dc 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -8,22 +8,19 @@ class PolicyBypassChecker def initialize(security_policy:, project:, user_access:, branch_name:) @security_policy = security_policy @project = project - @user_access = user_access + @user = user_access.user @branch_name = branch_name end - def check_bypass! - return false if bypass_settings.blank? - + def bypass_allowed? bypass_with_access_token? || bypass_with_service_account? end def bypass_with_access_token? - policy_token_ids = bypass_settings[:access_tokens]&.pluck(:id) + policy_token_ids = security_policy.bypass_settings.access_token_ids return false if policy_token_ids.blank? - user = user_access.user - return false unless user && user.project_bot? + return false unless user&.project_bot? user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key return false if user_token_ids.blank? @@ -33,11 +30,10 @@ def bypass_with_access_token? end def bypass_with_service_account? - policy_service_account_ids = bypass_settings[:service_accounts]&.pluck(:id) + policy_service_account_ids = security_policy.bypass_settings.service_account_ids return false if policy_service_account_ids.blank? - user = user_access.user - return false unless user && user.service_account? + return false unless user&.service_account? return false unless policy_service_account_ids.include?(user.id) log_bypass_audit!(:service_account, user.id) @@ -46,17 +42,12 @@ def bypass_with_service_account? private - attr_reader :security_policy, :project, :user_access, :branch_name - - def bypass_settings - security_policy.policy_content[:bypass_settings] - end - strong_memoize_attr :bypass_settings + attr_reader :security_policy, :project, :user, :branch_name def log_bypass_audit!(type, id) Gitlab::Audit::Auditor.audit( name: "security_policy_#{type}_push_bypass", - author: user_access.user, + author: user, scope: project, target: security_policy, message: diff --git a/ee/lib/security/scan_result_policies/push_bypass_checker.rb b/ee/lib/security/scan_result_policies/push_bypass_checker.rb index e4a6a2d6d3c2bd..92d6dc600869d0 100644 --- a/ee/lib/security/scan_result_policies/push_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/push_bypass_checker.rb @@ -30,7 +30,7 @@ def bypass_allowed?(policy) project: project, user_access: user_access, branch_name: branch_name - ).check_bypass! + ).bypass_allowed? end end end diff --git a/ee/spec/factories/security/policies.rb b/ee/spec/factories/security/policies.rb index 57d7504ff4c802..239fdaa90f214f 100644 --- a/ee/spec/factories/security/policies.rb +++ b/ee/spec/factories/security/policies.rb @@ -47,6 +47,27 @@ end end + transient do + bypass_access_token_ids { [] } + bypass_service_account_ids { [] } + end + + after(:build) do |policy, evaluator| + next if evaluator.bypass_access_token_ids.blank? + + policy.content ||= {} + policy.content[:bypass_settings] ||= {} + policy.content[:bypass_settings][:access_tokens] = evaluator.bypass_access_token_ids.map do |token_id| + { id: token_id } + end + + next if evaluator.bypass_service_account_ids.blank? + + policy.content[:bypass_settings] ||= {} + policy.content[:bypass_settings][:service_accounts] = evaluator + .bypass_service_account_ids.map { |service_account_id| { id: service_account_id } } + end + trait :deleted do policy_index { -1 } end diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index 72c3615cc67267..f8c8960cbd3409 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -107,11 +107,7 @@ let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } before do - create(:security_policy, linked_projects: [project], - content: { - bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } - } - ) + create(:security_policy, linked_projects: [project], bypass_access_token_ids: [personal_access_token.id]) end it 'does not raise' do diff --git a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb new file mode 100644 index 00000000000000..cc5076de79efe1 --- /dev/null +++ b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanResultPolicies::BypassSettings, feature_category: :security_policy_management do + let_it_be(:security_policy) do + build(:security_policy, bypass_access_token_ids: [1, 2], bypass_service_account_ids: [10, 20]) + end + + subject(:bypass_settings_instance) { described_class.new(security_policy) } + + describe '#access_token_ids' do + it 'returns the ids of access tokens' do + expect(bypass_settings_instance.access_token_ids).to eq([1, 2]) + end + + context 'when access_tokens is nil' do + let_it_be(:security_policy) { build(:security_policy, content: { bypass_settings: {} }) } + + it 'returns nil' do + expect(bypass_settings_instance.access_token_ids).to be_nil + end + end + end + + describe '#service_account_ids' do + it 'returns the ids of service accounts' do + expect(bypass_settings_instance.service_account_ids).to eq([10, 20]) + end + + context 'when service_accounts is nil' do + let_it_be(:security_policy) { build(:security_policy, content: { bypass_settings: {} }) } + + it 'returns nil' do + expect(bypass_settings_instance.service_account_ids).to be_nil + end + end + end +end diff --git a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb index e2c827290b8b49..375f3276967275 100644 --- a/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/policy_bypass_checker_spec.rb @@ -15,11 +15,11 @@ let_it_be(:service_account_access) { Gitlab::UserAccess.new(service_account, container: project) } let_it_be(:user_access) { Gitlab::UserAccess.new(user, container: project) } - describe '#check_bypass!' do - subject(:check_bypass!) do + describe '#bypass_allowed?' do + subject(:bypass_allowed?) do described_class.new( security_policy: security_policy, project: project, user_access: user_access, branch_name: branch_name - ).check_bypass! + ).bypass_allowed? end before do @@ -28,7 +28,7 @@ shared_examples 'bypass is not allowed and audit log is not created' do it 'returns false and does not create an audit log' do - result = check_bypass! + result = bypass_allowed? expect(result).to be false expect(Gitlab::Audit::Auditor).not_to have_received(:audit).with( @@ -66,7 +66,7 @@ end it 'returns true and creates an audit log' do - result = check_bypass! + result = bypass_allowed? expect(result).to be true expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( @@ -112,7 +112,7 @@ let_it_be(:user_access) { service_account_access } it 'returns true and creates an audit log' do - result = check_bypass! + result = bypass_allowed? expect(result).to be true expect(Gitlab::Audit::Auditor).to have_received(:audit).with(hash_including( diff --git a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb index a0ee3868ea1568..302a507c2bf88f 100644 --- a/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/push_bypass_checker_spec.rb @@ -35,9 +35,8 @@ context 'when there is a policy with bypass settings for access token' do let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } let_it_be_with_reload(:security_policy) do - create(:security_policy, :approval_policy, linked_projects: [project], content: { - bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } - }) + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [personal_access_token.id]) end it 'returns true' do @@ -60,16 +59,14 @@ let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } context 'when multiple policies have bypass_settings' do - let_it_be_with_reload(:security_policy1) do - create(:security_policy, :approval_policy, linked_projects: [project], content: { - bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } - }) + let_it_be_with_reload(:security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [personal_access_token.id]) end - let_it_be_with_reload(:security_policy2) do - create(:security_policy, :approval_policy, linked_projects: [project], content: { - bypass_settings: { access_tokens: [{ id: 999_999 }] } # not matching - }) + let_it_be_with_reload(:non_matching_security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [999_999]) end it 'returns true if any policy allows bypass' do @@ -78,13 +75,12 @@ end context 'when only one policy has bypass_settings' do - let_it_be_with_reload(:security_policy1) do - create(:security_policy, :approval_policy, linked_projects: [project], content: { - bypass_settings: { access_tokens: [{ id: personal_access_token.id }] } - }) + let_it_be_with_reload(:security_policy) do + create(:security_policy, :approval_policy, linked_projects: [project], + bypass_access_token_ids: [personal_access_token.id]) end - let_it_be_with_reload(:security_policy2) do + let_it_be_with_reload(:non_matching_security_policy) do create(:security_policy, :approval_policy, linked_projects: [project], content: {}) end diff --git a/ee/spec/models/security/policy_spec.rb b/ee/spec/models/security/policy_spec.rb index 1d05ddc0a70730..ff2e204c1958c3 100644 --- a/ee/spec/models/security/policy_spec.rb +++ b/ee/spec/models/security/policy_spec.rb @@ -1262,7 +1262,7 @@ describe '.with_bypass_settings' do let_it_be(:policy_with_bypass) do - create(:security_policy, content: { bypass_settings: { access_tokens: [{ id: 1 }] } }) + create(:security_policy, bypass_access_token_ids: [1]) end let_it_be(:policy_without_bypass) do -- GitLab From 6db168cbedecba5974734f3910eae0dd47150940 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Thu, 10 Jul 2025 19:45:57 +0200 Subject: [PATCH 21/23] Use match_array instead of eq --- .../lib/security/scan_result_policies/bypass_settings_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb index cc5076de79efe1..4bf31d9add8ffa 100644 --- a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb @@ -11,7 +11,7 @@ describe '#access_token_ids' do it 'returns the ids of access tokens' do - expect(bypass_settings_instance.access_token_ids).to eq([1, 2]) + expect(bypass_settings_instance.access_token_ids).to match_array([1, 2]) end context 'when access_tokens is nil' do @@ -25,7 +25,7 @@ describe '#service_account_ids' do it 'returns the ids of service accounts' do - expect(bypass_settings_instance.service_account_ids).to eq([10, 20]) + expect(bypass_settings_instance.service_account_ids).to match_array([10, 20]) end context 'when service_accounts is nil' do -- GitLab From 73f0f822e445f8119dee26e04d1cd3dd3aafd657 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Fri, 11 Jul 2025 14:40:33 +0200 Subject: [PATCH 22/23] Address review comments --- ...curity_policy_access_token_push_bypass.yml | 3 +- ...ity_policy_service_account_push_bypass.yml | 3 +- ee/app/models/security/policy.rb | 2 +- .../scan_result_policies/bypass_settings.rb | 4 +- .../policy_bypass_checker.rb | 6 ++- .../checks/security/policy_check_spec.rb | 26 ++++++++++--- .../bypass_settings_spec.rb | 19 +++++++--- ee/spec/models/security/policy_spec.rb | 37 +++++++++++++++++++ 8 files changed, 81 insertions(+), 19 deletions(-) diff --git a/config/audit_events/types/security_policy_access_token_push_bypass.yml b/config/audit_events/types/security_policy_access_token_push_bypass.yml index c0c8e4662379f6..b6a6830fdedfe5 100644 --- a/config/audit_events/types/security_policy_access_token_push_bypass.yml +++ b/config/audit_events/types/security_policy_access_token_push_bypass.yml @@ -1,7 +1,6 @@ --- name: security_policy_access_token_push_bypass -description: Branch push that is blocked by a security policy is bypassed for configured - access token +description: Branch push that is blocked by a security policy is bypassed for configured access token introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549644 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249 feature_category: security_policy_management diff --git a/config/audit_events/types/security_policy_service_account_push_bypass.yml b/config/audit_events/types/security_policy_service_account_push_bypass.yml index d497d696fa906c..926a02cecc72da 100644 --- a/config/audit_events/types/security_policy_service_account_push_bypass.yml +++ b/config/audit_events/types/security_policy_service_account_push_bypass.yml @@ -1,7 +1,6 @@ --- name: security_policy_service_account_push_bypass -description: Branch push that is blocked by a security policy is bypassed for configured - service account +description: Branch push that is blocked by a security policy is bypassed for configured service account introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/549644 introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/196249 feature_category: security_policy_management diff --git a/ee/app/models/security/policy.rb b/ee/app/models/security/policy.rb index e2034f4a46945a..604842faca364f 100644 --- a/ee/app/models/security/policy.rb +++ b/ee/app/models/security/policy.rb @@ -319,7 +319,7 @@ def enforced_scans=(scans) end def bypass_settings - Security::ScanResultPolicies::BypassSettings.new(self) + Security::ScanResultPolicies::BypassSettings.new(policy_content[:bypass_settings]) end strong_memoize_attr :bypass_settings diff --git a/ee/lib/security/scan_result_policies/bypass_settings.rb b/ee/lib/security/scan_result_policies/bypass_settings.rb index 4e9af3b3d801be..7022d805871b29 100644 --- a/ee/lib/security/scan_result_policies/bypass_settings.rb +++ b/ee/lib/security/scan_result_policies/bypass_settings.rb @@ -5,8 +5,8 @@ module ScanResultPolicies class BypassSettings include Gitlab::Utils::StrongMemoize - def initialize(security_policy) - @bypass_settings = security_policy.policy_content[:bypass_settings] || {} + def initialize(bypass_settings) + @bypass_settings = bypass_settings || {} end def access_token_ids diff --git a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb index 22a4fc10b379dc..69f68b3278502a 100644 --- a/ee/lib/security/scan_result_policies/policy_bypass_checker.rb +++ b/ee/lib/security/scan_result_policies/policy_bypass_checker.rb @@ -13,6 +13,8 @@ def initialize(security_policy:, project:, user_access:, branch_name:) end def bypass_allowed? + return false unless user + bypass_with_access_token? || bypass_with_service_account? end @@ -20,7 +22,7 @@ def bypass_with_access_token? policy_token_ids = security_policy.bypass_settings.access_token_ids return false if policy_token_ids.blank? - return false unless user&.project_bot? + return false unless user.project_bot? user_token_ids = user.personal_access_tokens.active.id_in(policy_token_ids).pluck_primary_key return false if user_token_ids.blank? @@ -33,7 +35,7 @@ def bypass_with_service_account? policy_service_account_ids = security_policy.bypass_settings.service_account_ids return false if policy_service_account_ids.blank? - return false unless user&.service_account? + return false unless user.service_account? return false unless policy_service_account_ids.include?(user.id) log_bypass_audit!(:service_account, user.id) diff --git a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb index f8c8960cbd3409..c242699ded0f27 100644 --- a/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb +++ b/ee/spec/lib/gitlab/checks/security/policy_check_spec.rb @@ -102,16 +102,32 @@ end end - context 'when policies_bypass_applied? allows bypass with access token' do + context 'with bypass_settings' do let_it_be(:user) { create(:user, :project_bot) } let_it_be(:personal_access_token) { create(:personal_access_token, user: user) } - before do - create(:security_policy, linked_projects: [project], bypass_access_token_ids: [personal_access_token.id]) + context 'when policies_bypass_applied? allows bypass with access token' do + before do + create(:security_policy, linked_projects: [project], bypass_access_token_ids: [personal_access_token.id]) + end + + it 'does not raise' do + expect { policy_check! }.not_to raise_error + end end - it 'does not raise' do - expect { policy_check! }.not_to raise_error + context 'when policies_bypass_applied? does not allow bypass with access token' do + before do + # Create a policy with a different access token, so the current token is not allowed + another_token = create(:personal_access_token) + create(:security_policy, linked_projects: [project], bypass_access_token_ids: [another_token.id]) + end + + it 'raises error' do + expect { policy_check! }.to raise_error( + Gitlab::GitAccess::ForbiddenError, described_class::FORCE_PUSH_ERROR_MESSAGE + ) + end end end diff --git a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb index 4bf31d9add8ffa..a5edd9e29c3fb2 100644 --- a/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb +++ b/ee/spec/lib/security/scan_result_policies/bypass_settings_spec.rb @@ -3,11 +3,20 @@ require 'spec_helper' RSpec.describe Security::ScanResultPolicies::BypassSettings, feature_category: :security_policy_management do - let_it_be(:security_policy) do - build(:security_policy, bypass_access_token_ids: [1, 2], bypass_service_account_ids: [10, 20]) + let_it_be(:bypass_settings) do + { + access_tokens: [ + { id: 1 }, + { id: 2 } + ], + service_accounts: [ + { id: 10 }, + { id: 20 } + ] + } end - subject(:bypass_settings_instance) { described_class.new(security_policy) } + subject(:bypass_settings_instance) { described_class.new(bypass_settings) } describe '#access_token_ids' do it 'returns the ids of access tokens' do @@ -15,7 +24,7 @@ end context 'when access_tokens is nil' do - let_it_be(:security_policy) { build(:security_policy, content: { bypass_settings: {} }) } + let_it_be(:bypass_settings) { {} } it 'returns nil' do expect(bypass_settings_instance.access_token_ids).to be_nil @@ -29,7 +38,7 @@ end context 'when service_accounts is nil' do - let_it_be(:security_policy) { build(:security_policy, content: { bypass_settings: {} }) } + let_it_be(:bypass_settings) { {} } it 'returns nil' do expect(bypass_settings_instance.service_account_ids).to be_nil diff --git a/ee/spec/models/security/policy_spec.rb b/ee/spec/models/security/policy_spec.rb index ff2e204c1958c3..bab7571c489b04 100644 --- a/ee/spec/models/security/policy_spec.rb +++ b/ee/spec/models/security/policy_spec.rb @@ -1276,4 +1276,41 @@ expect(result).to contain_exactly(policy_with_bypass) end end + + describe '#bypass_settings' do + let(:access_token_id) { 42 } + let(:service_account_id) { 99 } + + context 'when bypass_settings is nil' do + let(:policy) { build(:security_policy, content: {}) } + + it 'returns a BypassSettings object with nil ids' do + expect(policy.bypass_settings.access_token_ids).to be_nil + expect(policy.bypass_settings.service_account_ids).to be_nil + end + end + + context 'when bypass_settings is empty' do + let(:policy) { build(:security_policy, content: { bypass_settings: {} }) } + + it 'returns a BypassSettings object with nil ids' do + expect(policy.bypass_settings.access_token_ids).to be_nil + expect(policy.bypass_settings.service_account_ids).to be_nil + end + end + + context 'when bypass_settings has access_tokens and service_accounts' do + let(:policy) do + build(:security_policy, + bypass_access_token_ids: [access_token_id], + bypass_service_account_ids: [service_account_id] + ) + end + + it 'returns the correct ids' do + expect(policy.bypass_settings.access_token_ids).to contain_exactly(access_token_id) + expect(policy.bypass_settings.service_account_ids).to contain_exactly(service_account_id) + end + end + end end -- GitLab From e8da1f7f8c910d689096a0efa23eccbd44ffc892 Mon Sep 17 00:00:00 2001 From: Sashi Kumar Kumaresan Date: Fri, 11 Jul 2025 16:26:57 +0200 Subject: [PATCH 23/23] Refactor: Move pipeline_execution_policies classes to lib --- .../chain/pipeline_execution_policies/apply_policies.rb | 2 +- ee/lib/ee/gitlab/ci/variables/builder.rb | 2 +- .../pipeline/pipeline_execution_policies/pipeline_context.rb | 2 +- .../orchestration/project_pipeline_execution_policies.rb | 2 +- .../security/pipeline_execution_policies}/config.rb | 2 +- .../security/pipeline_execution_policies}/pipeline.rb | 2 +- .../security/pipeline_execution_policies}/usage_tracking.rb | 2 +- .../pipeline_execution_policies}/variables_override.rb | 4 ++-- .../factories/security/pipeline_execution_policy_configs.rb | 2 +- .../factories/security/pipeline_execution_policy_pipeline.rb | 2 +- .../security/pipeline_execution_policies}/config_spec.rb | 2 +- .../security/pipeline_execution_policies}/pipeline_spec.rb | 2 +- .../pipeline_execution_policies}/usage_tracking_spec.rb | 2 +- .../pipeline_execution_policies}/variables_override_spec.rb | 2 +- 14 files changed, 15 insertions(+), 15 deletions(-) rename ee/{app/models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/config.rb (98%) rename ee/{app/models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/pipeline.rb (93%) rename ee/{app/models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/usage_tracking.rb (97%) rename ee/{app/models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/variables_override.rb (94%) rename ee/spec/{models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/config_spec.rb (97%) rename ee/spec/{models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/pipeline_spec.rb (94%) rename ee/spec/{models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/usage_tracking_spec.rb (97%) rename ee/spec/{models/security/pipeline_execution_policy => lib/security/pipeline_execution_policies}/variables_override_spec.rb (97%) diff --git a/ee/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies.rb b/ee/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies.rb index c952912e0281b0..e89d3649171d08 100644 --- a/ee/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies.rb +++ b/ee/lib/ee/gitlab/ci/pipeline/chain/pipeline_execution_policies/apply_policies.rb @@ -84,7 +84,7 @@ def declared_stages end def usage_tracking - @usage_tracking ||= ::Security::PipelineExecutionPolicy::UsageTracking.new( + @usage_tracking ||= ::Security::PipelineExecutionPolicies::UsageTracking.new( project: project, policy_pipelines: command.pipeline_policy_context.policy_pipelines ) diff --git a/ee/lib/ee/gitlab/ci/variables/builder.rb b/ee/lib/ee/gitlab/ci/variables/builder.rb index a4d194f4115884..ac2fd070369586 100644 --- a/ee/lib/ee/gitlab/ci/variables/builder.rb +++ b/ee/lib/ee/gitlab/ci/variables/builder.rb @@ -48,7 +48,7 @@ def scoped_variables(job, environment:, dependencies:) def user_defined_variables( options:, environment:, job_variables: nil, expose_group_variables: protected_ref?, expose_project_variables: protected_ref?) - ::Security::PipelineExecutionPolicy::VariablesOverride.new(project: project, job_options: options) + ::Security::PipelineExecutionPolicies::VariablesOverride.new(project: project, job_options: options) .apply_variables_override(super) end end diff --git a/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb b/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb index 20dff68eed4024..77eb841f881222 100644 --- a/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb +++ b/ee/lib/gitlab/ci/pipeline/pipeline_execution_policies/pipeline_context.rb @@ -36,7 +36,7 @@ def build_policy_pipelines!(partition_id) pipeline = response.payload if response.success? - @policy_pipelines << ::Security::PipelineExecutionPolicy::Pipeline.new( + @policy_pipelines << ::Security::PipelineExecutionPolicies::Pipeline.new( pipeline: pipeline, policy_config: policy) elsif pipeline.filtered_as_empty? # no-op: we ignore empty pipelines diff --git a/ee/lib/gitlab/security/orchestration/project_pipeline_execution_policies.rb b/ee/lib/gitlab/security/orchestration/project_pipeline_execution_policies.rb index 8095b44a23da8d..aae3a69e793995 100644 --- a/ee/lib/gitlab/security/orchestration/project_pipeline_execution_policies.rb +++ b/ee/lib/gitlab/security/orchestration/project_pipeline_execution_policies.rb @@ -22,7 +22,7 @@ def configs .first(policy_limit) .reverse # reverse the order to apply the policy highest in the hierarchy as last .map do |(policy, policy_project_id, index)| - ::Security::PipelineExecutionPolicy::Config.new( + ::Security::PipelineExecutionPolicies::Config.new( policy: policy, policy_project_id: policy_project_id, policy_index: index) end end diff --git a/ee/app/models/security/pipeline_execution_policy/config.rb b/ee/lib/security/pipeline_execution_policies/config.rb similarity index 98% rename from ee/app/models/security/pipeline_execution_policy/config.rb rename to ee/lib/security/pipeline_execution_policies/config.rb index ce1e9db6b87a45..8dc1360b33effa 100644 --- a/ee/app/models/security/pipeline_execution_policy/config.rb +++ b/ee/lib/security/pipeline_execution_policies/config.rb @@ -2,7 +2,7 @@ # This class is the object representation of a single entry in the policy.yml module Security - module PipelineExecutionPolicy + module PipelineExecutionPolicies class Config include Gitlab::Utils::StrongMemoize include ::Security::PolicyCiSkippable diff --git a/ee/app/models/security/pipeline_execution_policy/pipeline.rb b/ee/lib/security/pipeline_execution_policies/pipeline.rb similarity index 93% rename from ee/app/models/security/pipeline_execution_policy/pipeline.rb rename to ee/lib/security/pipeline_execution_policies/pipeline.rb index 64b8d564f7cedc..5717360d7ebec3 100644 --- a/ee/app/models/security/pipeline_execution_policy/pipeline.rb +++ b/ee/lib/security/pipeline_execution_policies/pipeline.rb @@ -2,7 +2,7 @@ # This class is the object representation of a pipeline created by a policy module Security - module PipelineExecutionPolicy + module PipelineExecutionPolicies class Pipeline def initialize(pipeline:, policy_config:) @pipeline = pipeline diff --git a/ee/app/models/security/pipeline_execution_policy/usage_tracking.rb b/ee/lib/security/pipeline_execution_policies/usage_tracking.rb similarity index 97% rename from ee/app/models/security/pipeline_execution_policy/usage_tracking.rb rename to ee/lib/security/pipeline_execution_policies/usage_tracking.rb index c89eb3e217cd22..b606164a0aa6b9 100644 --- a/ee/app/models/security/pipeline_execution_policy/usage_tracking.rb +++ b/ee/lib/security/pipeline_execution_policies/usage_tracking.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Security - module PipelineExecutionPolicy + module PipelineExecutionPolicies class UsageTracking include ::Gitlab::InternalEventsTracking diff --git a/ee/app/models/security/pipeline_execution_policy/variables_override.rb b/ee/lib/security/pipeline_execution_policies/variables_override.rb similarity index 94% rename from ee/app/models/security/pipeline_execution_policy/variables_override.rb rename to ee/lib/security/pipeline_execution_policies/variables_override.rb index 110f856ebdc628..349e3bd742bbc7 100644 --- a/ee/app/models/security/pipeline_execution_policy/variables_override.rb +++ b/ee/lib/security/pipeline_execution_policies/variables_override.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Security - module PipelineExecutionPolicy + module PipelineExecutionPolicies class VariablesOverride def initialize(project:, job_options:) @project = project @@ -14,7 +14,7 @@ def initialize(project:, job_options:) def apply_highest_precedence(variables, yaml_variables) return variables unless apply_highest_precedence? - yaml_variable_keys = yaml_variables.pluck(:key).to_set # rubocop: disable Database/AvoidUsingPluckWithoutLimit -- this is not a DB query + yaml_variable_keys = yaml_variables.pluck(:key).to_set # rubocop: disable CodeReuse/ActiveRecord -- this is not a DB query variables.reject { |var| yaml_variable_keys.include?(var.key) }.concat(yaml_variables) end diff --git a/ee/spec/factories/security/pipeline_execution_policy_configs.rb b/ee/spec/factories/security/pipeline_execution_policy_configs.rb index fbd640d7ffa4b3..77a948407a4cee 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_configs.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_configs.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory( :pipeline_execution_policy_config, - class: '::Security::PipelineExecutionPolicy::Config' + class: '::Security::PipelineExecutionPolicies::Config' ) do policy factory: :pipeline_execution_policy policy_project_id { 123456 } diff --git a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb index ce6ce1bb49e458..76407ed60dd250 100644 --- a/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb +++ b/ee/spec/factories/security/pipeline_execution_policy_pipeline.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory( :pipeline_execution_policy_pipeline, - class: '::Security::PipelineExecutionPolicy::Pipeline' + class: '::Security::PipelineExecutionPolicies::Pipeline' ) do pipeline factory: :ci_empty_pipeline policy_config factory: :pipeline_execution_policy_config diff --git a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb b/ee/spec/lib/security/pipeline_execution_policies/config_spec.rb similarity index 97% rename from ee/spec/models/security/pipeline_execution_policy/config_spec.rb rename to ee/spec/lib/security/pipeline_execution_policies/config_spec.rb index 604c4e4c7a61ef..9afdb59cc9f0d9 100644 --- a/ee/spec/models/security/pipeline_execution_policy/config_spec.rb +++ b/ee/spec/lib/security/pipeline_execution_policies/config_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Security::PipelineExecutionPolicy::Config, feature_category: :security_policy_management do +RSpec.describe Security::PipelineExecutionPolicies::Config, feature_category: :security_policy_management do let(:config) { described_class.new(**params) } let(:params) { { policy_project_id: 123, policy_index: 1, policy: policy } } diff --git a/ee/spec/models/security/pipeline_execution_policy/pipeline_spec.rb b/ee/spec/lib/security/pipeline_execution_policies/pipeline_spec.rb similarity index 94% rename from ee/spec/models/security/pipeline_execution_policy/pipeline_spec.rb rename to ee/spec/lib/security/pipeline_execution_policies/pipeline_spec.rb index e2b5592f1dedb9..d6efff164b109f 100644 --- a/ee/spec/models/security/pipeline_execution_policy/pipeline_spec.rb +++ b/ee/spec/lib/security/pipeline_execution_policies/pipeline_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Security::PipelineExecutionPolicy::Pipeline, feature_category: :security_policy_management do +RSpec.describe Security::PipelineExecutionPolicies::Pipeline, feature_category: :security_policy_management do let(:policy_config) { build(:pipeline_execution_policy_config) } let(:instance) { described_class.new(pipeline: build(:ci_empty_pipeline), policy_config: policy_config) } diff --git a/ee/spec/models/security/pipeline_execution_policy/usage_tracking_spec.rb b/ee/spec/lib/security/pipeline_execution_policies/usage_tracking_spec.rb similarity index 97% rename from ee/spec/models/security/pipeline_execution_policy/usage_tracking_spec.rb rename to ee/spec/lib/security/pipeline_execution_policies/usage_tracking_spec.rb index 8f78a720285157..964a69fc526182 100644 --- a/ee/spec/models/security/pipeline_execution_policy/usage_tracking_spec.rb +++ b/ee/spec/lib/security/pipeline_execution_policies/usage_tracking_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Security::PipelineExecutionPolicy::UsageTracking, feature_category: :security_policy_management do +RSpec.describe Security::PipelineExecutionPolicies::UsageTracking, feature_category: :security_policy_management do using RSpec::Parameterized::TableSyntax let_it_be(:group) { create(:group) } diff --git a/ee/spec/models/security/pipeline_execution_policy/variables_override_spec.rb b/ee/spec/lib/security/pipeline_execution_policies/variables_override_spec.rb similarity index 97% rename from ee/spec/models/security/pipeline_execution_policy/variables_override_spec.rb rename to ee/spec/lib/security/pipeline_execution_policies/variables_override_spec.rb index ecc9a9ae8e483e..0c4b541b1a79d6 100644 --- a/ee/spec/models/security/pipeline_execution_policy/variables_override_spec.rb +++ b/ee/spec/lib/security/pipeline_execution_policies/variables_override_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Security::PipelineExecutionPolicy::VariablesOverride, feature_category: :security_policy_management do +RSpec.describe Security::PipelineExecutionPolicies::VariablesOverride, feature_category: :security_policy_management do let_it_be(:project) { create(:project) } let(:variables_override) { described_class.new(project: project, job_options: job_options) } -- GitLab