From a746c0cee1bd716ea79a8183242108ffbc198a13 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 5 Dec 2025 12:46:05 -0500 Subject: [PATCH 01/10] Add security scan profile attach mutation Changelog: added EE: true --- app/models/namespace.rb | 1 + app/models/project.rb | 3 + .../security_scan_profiles/apply.yml | 4 + doc/api/graphql/reference/_index.md | 27 ++ doc/user/custom_roles/abilities.md | 1 + ee/app/graphql/ee/types/mutation_type.rb | 1 + .../security/scan_profiles/attach.rb | 100 ++++++ .../models/security/scan_profile_project.rb | 8 + ee/app/policies/ee/group_policy.rb | 7 + ee/app/policies/ee/project_policy.rb | 7 + .../security/scan_profiles/attach_service.rb | 84 +++++ .../scan_profiles/find_or_create_service.rb | 51 +++ .../apply_security_scan_profiles.yml | 12 + .../wip/security_scan_profiles.yml | 10 + .../security/scan_profile_project_spec.rb | 74 ++++- ee/spec/models/security/scan_profile_spec.rb | 18 ++ ee/spec/policies/group_policy_spec.rb | 54 ++++ ee/spec/policies/project_policy_spec.rb | 44 +++ .../security/scan_profiles/attach_spec.rb | 301 ++++++++++++++++++ .../scan_profiles/attach_service_spec.rb | 207 ++++++++++++ .../find_or_create_service_spec.rb | 129 ++++++++ spec/models/namespace_spec.rb | 10 + spec/models/project_spec.rb | 16 + 23 files changed, 1153 insertions(+), 16 deletions(-) create mode 100644 config/authz/permissions/security_scan_profiles/apply.yml create mode 100644 ee/app/graphql/mutations/security/scan_profiles/attach.rb create mode 100644 ee/app/services/security/scan_profiles/attach_service.rb create mode 100644 ee/app/services/security/scan_profiles/find_or_create_service.rb create mode 100644 ee/config/custom_abilities/apply_security_scan_profiles.yml create mode 100644 ee/config/feature_flags/wip/security_scan_profiles.yml create mode 100644 ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb create mode 100644 ee/spec/services/security/scan_profiles/attach_service_spec.rb create mode 100644 ee/spec/services/security/scan_profiles/find_or_create_service_spec.rb diff --git a/app/models/namespace.rb b/app/models/namespace.rb index cf9077f3f8bd52..7cc7912bd2710b 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -230,6 +230,7 @@ class Namespace < ApplicationRecord scope :by_parent, ->(parent) { where(parent_id: parent) } scope :by_root_id, ->(root_id) { where('traversal_ids[1] IN (?)', root_id) } scope :by_not_in_root_id, ->(root_id) { where('namespaces.traversal_ids[1] NOT IN (?)', root_id) } + scope :root_namespace_ids, -> { limit(MAX_PLUCK).pluck(Arel.sql('traversal_ids[1]')).uniq } scope :filter_by_path, ->(query) { where('lower(path) = :query', query: query.downcase) } scope :in_organization, ->(organization) { where(organization: organization) } scope :by_name, ->(name) { where('name LIKE ?', "#{sanitize_sql_like(name)}%") } diff --git a/app/models/project.rb b/app/models/project.rb index 5d55da4dd0e016..012b5fb2ea2e87 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -677,6 +677,9 @@ def with_developer_access scope :not_in_groups, ->(groups) { where.not(group: groups) } scope :by_not_in_root_id, ->(root_id) { joins(:project_namespace).where('namespaces.traversal_ids[1] NOT IN (?)', root_id) } scope :with_visibility_level_greater_than, ->(level) { where("visibility_level > ?", level) } + scope :root_namespace_ids, -> { + joins(:namespace).limit(MAX_PLUCK).pluck(Arel.sql('namespaces.traversal_ids[1]')).uniq + } scope :aimed_for_deletion, -> { where.not(marked_for_deletion_at: nil).without_deleted } scope :self_or_ancestors_aimed_for_deletion, -> do diff --git a/config/authz/permissions/security_scan_profiles/apply.yml b/config/authz/permissions/security_scan_profiles/apply.yml new file mode 100644 index 00000000000000..e3db4b4f3296b8 --- /dev/null +++ b/config/authz/permissions/security_scan_profiles/apply.yml @@ -0,0 +1,4 @@ +--- +name: apply_security_scan_profiles +description: Apply security scan profiles. +feature_category: security_asset_inventories diff --git a/doc/api/graphql/reference/_index.md b/doc/api/graphql/reference/_index.md index 9d669ffed4fe39..75b2b7740e00de 100644 --- a/doc/api/graphql/reference/_index.md +++ b/doc/api/graphql/reference/_index.md @@ -12417,6 +12417,31 @@ Input type: `SecurityPolicyProjectUnassignInput` | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | `errors` | [`[String!]!`](#string) | Errors encountered during the mutation. | +### `Mutation.securityScanProfileAttach` + +{{< details >}} +**Introduced** in GitLab 18.7. +**Status**: Experiment. +{{< /details >}} + +Input type: `SecurityScanProfileAttachInput` + +#### Arguments + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `groupIds` | [`[GroupID!]`](#groupid) | Group IDs to attach the profile to. | +| `projectIds` | [`[ProjectID!]`](#projectid) | Project IDs to attach the profile to. | +| `securityScanProfileId` | [`SecurityScanProfileID!`](#securityscanprofileid) | Security scan profile ID to attach. | + +#### Fields + +| Name | Type | Description | +| ---- | ---- | ----------- | +| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | +| `errors` | [`[String!]!`](#string) | Errors encountered during the mutation. | + ### `Mutation.securityTrainingUpdate` Input type: `SecurityTrainingUpdateInput` @@ -52752,6 +52777,7 @@ Member role permission. | `ADMIN_TERRAFORM_STATE` | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | | `ADMIN_VULNERABILITY` | Edit the status, linked issue, and severity of a vulnerability object. Also requires the `read_vulnerability` permission. | | `ADMIN_WEB_HOOK` | Manage webhooks. | +| `APPLY_SECURITY_SCAN_PROFILES` | Apply security scan profiles. | | `ARCHIVE_PROJECT` | Allows archiving of projects. | | `MANAGE_DEPLOY_TOKENS` | Manage deploy tokens at the group or project level. | | `MANAGE_GROUP_ACCESS_TOKENS` | Create, read, update, and delete group access tokens. When creating a token, users with this custom permission must select a role for that token that has the same or fewer permissions as the default role used as the base for the custom role. | @@ -52796,6 +52822,7 @@ Member role standard permission. | `ADMIN_TERRAFORM_STATE` | Execute terraform commands, lock/unlock terraform state files, and remove file versions. | | `ADMIN_VULNERABILITY` | Edit the status, linked issue, and severity of a vulnerability object. Also requires the `read_vulnerability` permission. | | `ADMIN_WEB_HOOK` | Manage webhooks. | +| `APPLY_SECURITY_SCAN_PROFILES` | Apply security scan profiles. | | `ARCHIVE_PROJECT` | Allows archiving of projects. | | `MANAGE_DEPLOY_TOKENS` | Manage deploy tokens at the group or project level. | | `MANAGE_GROUP_ACCESS_TOKENS` | Create, read, update, and delete group access tokens. When creating a token, users with this custom permission must select a role for that token that has the same or fewer permissions as the default role used as the base for the custom role. | diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index 5c33d9ec1df2de..bc494482727e0f 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -102,6 +102,7 @@ Any dependencies are noted in the `Description` column for each permission. | Permission | Description | API Attribute | Scope | Introduced | |:-----------|:------------|:--------------|:------|:-----------| +| Apply security scan profiles | Apply security scan profiles. | [`apply_security_scan_profiles`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/999999999) | Group,
Project | GitLab [18.7](https://gitlab.com/groups/gitlab-org/-/epics/19802) | | Read security scan profiles | Read security scan profiles. | [`read_security_scan_profiles`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/213203) | Group,
Project | GitLab [18.7](https://gitlab.com/groups/gitlab-org/-/epics/19802) | ## Security policy management diff --git a/ee/app/graphql/ee/types/mutation_type.rb b/ee/app/graphql/ee/types/mutation_type.rb index 8b0f72b5548a7c..cf01147b0ee1b6 100644 --- a/ee/app/graphql/ee/types/mutation_type.rb +++ b/ee/app/graphql/ee/types/mutation_type.rb @@ -215,6 +215,7 @@ def self.authorization_scopes mount_mutation ::Mutations::Security::Categories::Create mount_mutation ::Mutations::Security::Categories::Update mount_mutation ::Mutations::Security::Categories::Destroy + mount_mutation ::Mutations::Security::ScanProfiles::Attach, experiment: { milestone: '18.7' } mount_mutation ::Mutations::Users::Abuse::NamespaceBans::Destroy mount_mutation ::Mutations::Users::MemberRoles::Assign, experiment: { milestone: '17.7' } mount_mutation ::Mutations::AuditEvents::ExternalAuditEventDestinations::Create diff --git a/ee/app/graphql/mutations/security/scan_profiles/attach.rb b/ee/app/graphql/mutations/security/scan_profiles/attach.rb new file mode 100644 index 00000000000000..954ecd197fccc9 --- /dev/null +++ b/ee/app/graphql/mutations/security/scan_profiles/attach.rb @@ -0,0 +1,100 @@ +# frozen_string_literal: true + +module Mutations + module Security + module ScanProfiles + class Attach < BaseMutation + graphql_name 'SecurityScanProfileAttach' + MAX_IDS = 100 + + argument :security_scan_profile_id, Types::GlobalIDType[::Security::ScanProfile], + required: true, + description: 'Security scan profile ID to attach.', + prepare: ->(global_id, _ctx) { global_id.model_id } + + argument :project_ids, [Types::GlobalIDType[::Project]], + required: false, + description: 'Project IDs to attach the profile to.', + prepare: ->(global_ids, _ctx) { global_ids.map(&:model_id) } + + argument :group_ids, [Types::GlobalIDType[::Group]], + required: false, + description: 'Group IDs to attach the profile to.', + prepare: ->(global_ids, _ctx) { global_ids.map(&:model_id) } + + def resolve(security_scan_profile_id:, project_ids: nil, group_ids: nil) + validate_id_limit!(project_ids, group_ids) + + root_namespace = shared_root_namespace!(project_ids, group_ids) + raise_resource_not_available_error! unless Feature.enabled?(:security_scan_profiles, root_namespace) + + authorized_projects = load_and_authorize_projects(project_ids) + authorized_groups = load_and_authorize_groups(group_ids) + if (authorized_projects.size != project_ids.to_a.size) || (authorized_groups.size != group_ids.to_a.size) + raise_resource_not_available_error! + end + + profile = resolve_profile(security_scan_profile_id, root_namespace) + raise_resource_not_available_error! unless profile + + result = ::Security::ScanProfiles::AttachService.execute( + profile: profile, + projects: authorized_projects, + groups: authorized_groups + ) + + { + errors: result[:errors] + } + end + + private + + def resolve_profile(security_scan_profile_id, root_namespace) + if Enums::Security.scan_profile_types.key?(security_scan_profile_id.to_sym) + profile_result = ::Security::ScanProfiles::FindOrCreateService.execute( + namespace: root_namespace, + scan_type: security_scan_profile_id.to_sym + ) + + profile_result.payload[:scan_profile] if profile_result.success? + else + ::Security::ScanProfile.by_namespace(root_namespace).id_in(security_scan_profile_id).first + end + end + + def load_and_authorize_projects(project_ids) + return [] unless project_ids&.any? + + Project.id_in(project_ids).select { |project| current_user.can?(:apply_security_scan_profiles, project) } + end + + def load_and_authorize_groups(group_ids) + return [] unless group_ids&.any? + + Group.id_in(group_ids).select { |group| current_user.can?(:apply_security_scan_profiles, group) } + end + + def validate_id_limit!(project_ids, group_ids) + total = project_ids.to_a.size + group_ids.to_a.size + return if total <= MAX_IDS + + raise Gitlab::Graphql::Errors::ArgumentError, "Too many ids (maximum: #{MAX_IDS})" + end + + def shared_root_namespace!(project_ids, group_ids) + root_namespace_ids = [] + root_namespace_ids += Project.id_in(project_ids).root_namespace_ids if project_ids&.any? + root_namespace_ids += Group.id_in(group_ids).root_namespace_ids if group_ids&.any? + root_namespace_ids.uniq! + + if root_namespace_ids.empty? || root_namespace_ids.size > 1 + raise Gitlab::Graphql::Errors::ArgumentError, "All items should belong to the same root namespace" + end + + Group.find(root_namespace_ids.first) + end + end + end + end +end diff --git a/ee/app/models/security/scan_profile_project.rb b/ee/app/models/security/scan_profile_project.rb index 664ba255481309..82872c30b5cab9 100644 --- a/ee/app/models/security/scan_profile_project.rb +++ b/ee/app/models/security/scan_profile_project.rb @@ -4,6 +4,8 @@ module Security class ScanProfileProject < ::SecApplicationRecord self.table_name = 'security_scan_profiles_projects' + MAX_PROFILES_PER_PROJECT = 10 + belongs_to :project, optional: false belongs_to :scan_profile, class_name: 'Security::ScanProfile', foreign_key: :security_scan_profile_id, inverse_of: :scan_profile_projects, optional: false @@ -13,5 +15,11 @@ class ScanProfileProject < ::SecApplicationRecord scope :not_in_root_namespace, ->(root_namespace) { joins(:scan_profile).where.not(security_scan_profiles: { namespace: root_namespace }) } + scope :by_scan_profile, ->(profile_id) { where(security_scan_profile_id: profile_id) } + scope :by_project, ->(project_ids) { where(project_id: project_ids) } + scope :project_ids_at_limit, -> { + group(:project_id).having('COUNT(*) >= ?', MAX_PROFILES_PER_PROJECT) + .limit(MAX_PLUCK).pluck(:project_id) + } end end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 94719f5e4862a9..188c72a5451928 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -729,8 +729,15 @@ module GroupPolicy rule { custom_role_enables_read_security_scan_profiles }.enable(:read_security_scan_profiles) + rule { can?(:maintainer_access) }.policy do + enable :apply_security_scan_profiles + end + + rule { custom_role_enables_apply_security_scan_profiles }.enable(:apply_security_scan_profiles) + rule { ~security_scan_profiles_available }.policy do prevent :read_security_scan_profiles + prevent :apply_security_scan_profiles end rule { ~security_inventory_available }.prevent :read_security_inventory diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 3d1c5f9582e27e..44f439ed15e75b 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -610,8 +610,15 @@ module ProjectPolicy rule { custom_role_enables_read_security_scan_profiles }.enable(:read_security_scan_profiles) + rule { can?(:maintainer_access) }.policy do + enable :apply_security_scan_profiles + end + + rule { custom_role_enables_apply_security_scan_profiles }.enable(:apply_security_scan_profiles) + rule { ~security_scan_profiles_available }.policy do prevent :read_security_scan_profiles + prevent :apply_security_scan_profiles end rule { ~validity_checks_available }.policy do diff --git a/ee/app/services/security/scan_profiles/attach_service.rb b/ee/app/services/security/scan_profiles/attach_service.rb new file mode 100644 index 00000000000000..18b4779272a1e2 --- /dev/null +++ b/ee/app/services/security/scan_profiles/attach_service.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module Security + module ScanProfiles + class AttachService + def self.execute(profile:, projects: [], groups: []) + new(profile: profile, projects: projects, groups: groups).execute + end + + def initialize(profile:, projects: [], groups: []) + @profile = profile + @projects = projects + @groups = groups + @errors = [] + end + + def execute + return error_result('At least one project or group must be provided') if projects.empty? && groups.empty? + + attach_to_projects + attach_to_groups + + { + errors: errors + } + rescue StandardError => e + error_result(e.message) + end + + attr_reader :profile, :projects, :groups, :errors + + private + + def attach_to_projects + return [] if projects.empty? + + project_ids_at_limit = Security::ScanProfileProject.by_project(projects.map(&:id)).project_ids_at_limit + projects_at_limit = projects.select { |p| project_ids_at_limit.include?(p.id) } + projects_to_attach = projects.reject { |p| project_ids_at_limit.include?(p.id) } + + projects_at_limit.each do |project| + errors << "Project #{project.id} has reached the maximum limit of scan profiles" + end + + bulk_attach_to_projects(projects_to_attach) if projects_to_attach.any? + end + + def bulk_attach_to_projects(projects_to_attach) + timestamp = Time.current + + records = projects_to_attach.map do |project| + { + project_id: project.id, + security_scan_profile_id: profile.id, + created_at: timestamp, + updated_at: timestamp + } + end + + Security::ScanProfileProject.upsert_all(records, unique_by: [:project_id, :security_scan_profile_id]) + groups + end + + def attach_to_groups + return [] if groups.empty? + + # groups.each do |group| + # SecurityScanProfiles::AttachWorker.perform_async( + # group.id, + # profile.id + # ) + # end + + groups # REMOVE - This is here so rubocop won't be mad + end + + def error_result(message) + { + errors: [message] + } + end + end + end +end diff --git a/ee/app/services/security/scan_profiles/find_or_create_service.rb b/ee/app/services/security/scan_profiles/find_or_create_service.rb new file mode 100644 index 00000000000000..426eaaee438fda --- /dev/null +++ b/ee/app/services/security/scan_profiles/find_or_create_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module Security + module ScanProfiles + class FindOrCreateService + def self.execute(namespace:, scan_type:) + new(namespace: namespace, scan_type: scan_type).execute + end + + def initialize(namespace:, scan_type:) + @namespace = namespace + @scan_type = scan_type + end + + def execute + return error_response('Namespace must be a root namespace') unless namespace.root? + + profile = Security::ScanProfile.by_namespace(namespace).by_type(scan_type).gitlab_recommended.first + return success_response(profile) if profile + + profile = create_profile + return error_response('Could not find a default scan profile for this type') unless profile.present? + + success_response(profile) + end + + private + + attr_reader :namespace, :scan_type + + def create_profile + default_profile = Security::DefaultScanProfilesHelper.default_scan_profiles.find do |profile| + profile.scan_type == scan_type.to_s + end + return unless default_profile + + default_profile.namespace = namespace + default_profile.save! + default_profile + end + + def success_response(profile) + ServiceResponse.success(payload: { scan_profile: profile }) + end + + def error_response(message) + ServiceResponse.error(message: message) + end + end + end +end diff --git a/ee/config/custom_abilities/apply_security_scan_profiles.yml b/ee/config/custom_abilities/apply_security_scan_profiles.yml new file mode 100644 index 00000000000000..ef3483f1360b30 --- /dev/null +++ b/ee/config/custom_abilities/apply_security_scan_profiles.yml @@ -0,0 +1,12 @@ +--- +title: Apply security scan profiles +name: apply_security_scan_profiles +description: Apply security scan profiles. +introduced_by_issue: https://gitlab.com/groups/gitlab-org/-/epics/19802 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/999999999 +feature_category: security_asset_inventories +milestone: '18.7' +group_ability: true +enabled_for_group_access_levels: [40, 50] +project_ability: true +enabled_for_project_access_levels: [40, 50] diff --git a/ee/config/feature_flags/wip/security_scan_profiles.yml b/ee/config/feature_flags/wip/security_scan_profiles.yml new file mode 100644 index 00000000000000..6165d6e16943ee --- /dev/null +++ b/ee/config/feature_flags/wip/security_scan_profiles.yml @@ -0,0 +1,10 @@ +--- +name: security_scan_profiles +description: +feature_issue_url: +introduced_by_url: +rollout_issue_url: +milestone: '18.7' +group: group::security platform management +type: wip +default_enabled: false diff --git a/ee/spec/models/security/scan_profile_project_spec.rb b/ee/spec/models/security/scan_profile_project_spec.rb index 7418568270d0b5..bfe1da31bdd0c1 100644 --- a/ee/spec/models/security/scan_profile_project_spec.rb +++ b/ee/spec/models/security/scan_profile_project_spec.rb @@ -5,7 +5,7 @@ RSpec.describe Security::ScanProfileProject, feature_category: :security_asset_inventories do let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, namespace: group) } - let_it_be(:scan_profile) { create(:security_scan_profile, namespace: group) } + let_it_be(:scan_profile) { create(:security_scan_profile, namespace: group, name: 'Profile 1') } describe 'associations' do it { is_expected.to belong_to(:scan_profile).class_name('Security::ScanProfile').required } @@ -19,33 +19,75 @@ end describe 'scopes' do - describe '.not_in_root_namespace' do - let_it_be(:root_namespace) { create(:group) } - let_it_be(:other_namespace) { create(:group) } - let_it_be(:project_in_root) { create(:project, namespace: root_namespace) } - let_it_be(:project_in_other) { create(:project, namespace: other_namespace) } + let_it_be(:root_namespace) { create(:group) } + let_it_be(:other_namespace) { create(:group) } - let_it_be(:scan_profile_in_root) { create(:security_scan_profile, namespace: root_namespace) } - let_it_be(:scan_profile_in_other) { create(:security_scan_profile, namespace: other_namespace) } + let_it_be(:project) { create(:project, namespace: root_namespace) } + let_it_be(:project2) { create(:project, namespace: root_namespace) } + let_it_be(:project_in_other) { create(:project, namespace: other_namespace) } - let_it_be(:association_in_root) do - create(:security_scan_profile_project, scan_profile: scan_profile_in_root, project: project_in_root) - end + let_it_be(:scan_profile) { create(:security_scan_profile, namespace: root_namespace) } + let_it_be(:scan_profile2) { create(:security_scan_profile, namespace: root_namespace, name: 'Profile 2') } + let_it_be(:scan_profile_in_other) { create(:security_scan_profile, namespace: other_namespace) } - let_it_be(:association_in_other) do - create(:security_scan_profile_project, scan_profile: scan_profile_in_other, project: project_in_other) - end + let_it_be(:profile_project1) do + create(:security_scan_profile_project, scan_profile: scan_profile, project: project) + end + + let_it_be(:profile_project2) do + create(:security_scan_profile_project, scan_profile: scan_profile2, project: project2) + end + let_it_be(:association_in_other) do + create(:security_scan_profile_project, scan_profile: scan_profile_in_other, project: project_in_other) + end + + describe '.not_in_root_namespace' do it 'returns associations where scan profile is not in the given root namespace' do result = described_class.not_in_root_namespace(root_namespace) expect(result).to contain_exactly(association_in_other) end - it 'returns empty when all associations are in the root namespace' do + it 'returns associations not in the specified namespace' do result = described_class.not_in_root_namespace(other_namespace) - expect(result).to contain_exactly(association_in_root) + expect(result).to contain_exactly(profile_project1, profile_project2) + end + end + + describe '.by_scan_profile' do + it 'returns correct associations' do + expect(described_class.by_scan_profile(scan_profile.id)).to contain_exactly(profile_project1) + expect(described_class.by_scan_profile(scan_profile2.id)).to contain_exactly(profile_project2) + expect(described_class.by_scan_profile(nil)).to be_empty + end + end + + describe '.by_project' do + it 'returns correct associations' do + expect(described_class.by_project(project.id)).to contain_exactly(profile_project1) + expect(described_class.by_project(project2.id)).to contain_exactly(profile_project2) + expect(described_class.by_project([project.id, project2.id])) + .to contain_exactly(profile_project1, profile_project2) + expect(described_class.by_project(nil)).to be_empty + end + end + + describe '.project_ids_at_limit' do + let_it_be(:project_at_limit) { create(:project, namespace: root_namespace) } + let_it_be(:project_under_limit) { create(:project, namespace: root_namespace) } + + before do + stub_const("#{described_class}::MAX_PROFILES_PER_PROJECT", 2) + + create(:security_scan_profile_project, scan_profile: scan_profile, project: project_at_limit) + create(:security_scan_profile_project, scan_profile: scan_profile2, project: project_at_limit) + create(:security_scan_profile_project, scan_profile: scan_profile, project: project_under_limit) + end + + it 'returns project ids that have reached the limit' do + expect(described_class.project_ids_at_limit).to contain_exactly(project_at_limit.id) end end end diff --git a/ee/spec/models/security/scan_profile_spec.rb b/ee/spec/models/security/scan_profile_spec.rb index c6534ea58fbab3..3fde66d379ae90 100644 --- a/ee/spec/models/security/scan_profile_spec.rb +++ b/ee/spec/models/security/scan_profile_spec.rb @@ -52,6 +52,24 @@ end end + describe 'scopes' do + describe '.gitlab_recommended' do + let_it_be(:profile) { create(:security_scan_profile, namespace: root_level_group, scan_type: :secret_detection) } + let_it_be(:gitlab_recommended_profile) do + create(:security_scan_profile, + namespace: root_level_group, + scan_type: :secret_detection, + gitlab_recommended: true, + name: "gitlab_recommended_profile" + ) + end + + it 'returns gitlab recommended profiles' do + expect(described_class.gitlab_recommended).to match_array([gitlab_recommended_profile]) + end + end + end + context 'with loose foreign key on security_scan_profiles.namespace_id' do it_behaves_like 'cleanup by a loose foreign key' do let_it_be(:parent) { root_level_group } diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 9663be459d6dac..994913212b124a 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -4475,6 +4475,16 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'does not call custom role query', [:developer, :maintainer, :owner] end + + context 'for a member role with the `apply_security_scan_profiles` ability' do + let(:licensed_features) { { security_scan_profiles: true } } + let(:member_role_abilities) { { apply_security_scan_profiles: true } } + let(:allowed_abilities) { [:apply_security_scan_profiles] } + + it_behaves_like 'custom roles abilities' + + it_behaves_like 'does not call custom role query', [:developer, :maintainer, :owner] + end end end end @@ -5276,6 +5286,50 @@ def create_member_role(member, abilities = member_role_abilities) end end + describe 'apply_security_scan_profiles' do + let(:policy) { :apply_security_scan_profiles } + + context 'when security_scan_profiles is available' do + before do + stub_licensed_features(security_scan_profiles: true) + enable_admin_mode!(current_user) if role == :admin + end + + where(:role, :allowed) do + :guest | false + :planner | false + :reporter | false + :developer | false + :maintainer | true + :owner | true + :admin | true + :auditor | false + end + + with_them do + let(:current_user) { public_send(role) } + + it { is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy)) } + end + end + + context 'when security_scan_profiles is not available' do + where(:role) do + [:guest, :planner, :reporter, :developer, :maintainer, :auditor, :owner, :admin] + end + + with_them do + let(:current_user) { public_send(role) } + + before do + enable_admin_mode!(current_user) if role == :admin + end + + it { is_expected.to be_disallowed(policy) } + end + end + end + describe 'AI catalog abilities' do let(:ai_catalog_available) { true } let(:flows_available) { true } diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 92f3f31a84d273..4040ccddfcac27 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -1097,6 +1097,50 @@ end end + describe 'apply_security_scan_profiles' do + let(:policy) { :apply_security_scan_profiles } + + context 'when security_scan_profiles is available' do + before do + stub_licensed_features(security_scan_profiles: true) + enable_admin_mode!(current_user) if role == :admin + end + + where(:role, :allowed) do + :guest | false + :planner | false + :reporter | false + :developer | false + :maintainer | true + :owner | true + :admin | true + :auditor | false + end + + with_them do + let(:current_user) { public_send(role) } + + it { is_expected.to(allowed ? be_allowed(policy) : be_disallowed(policy)) } + end + end + + context 'when security_scan_profiles is not available' do + where(:role) do + [:guest, :planner, :reporter, :developer, :maintainer, :auditor, :owner, :admin] + end + + with_them do + let(:current_user) { public_send(role) } + + before do + enable_admin_mode!(current_user) if role == :admin + end + + it { is_expected.to be_disallowed(policy) } + end + end + end + describe 'remove_project when default_project_deletion_protection is set to true' do before do stub_application_setting(default_project_deletion_protection: true) diff --git a/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb b/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb new file mode 100644 index 00000000000000..7b60b20feec606 --- /dev/null +++ b/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb @@ -0,0 +1,301 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'SecurityScanProfileAttach', feature_category: :security_asset_inventories do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:root_group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: root_group) } + let_it_be(:another_root_group) { create(:group) } + let_it_be(:project1) { create(:project, namespace: root_group) } + let_it_be(:project2) { create(:project, namespace: root_group) } + let_it_be(:subgroup_project) { create(:project, namespace: subgroup) } + let_it_be(:group1) { create(:group, parent: root_group) } + let_it_be(:profile) do + create(:security_scan_profile, + namespace: root_group, + scan_type: :secret_detection, + name: 'Test Profile') + end + + let(:security_scan_profile_id) { profile.to_global_id.to_s } + let(:project_ids) { [project1.to_global_id.to_s, project2.to_global_id.to_s] } + let(:group_ids) { nil } + + let(:mutation) do + graphql_mutation( + :security_scan_profile_attach, + { + security_scan_profile_id: security_scan_profile_id, + project_ids: project_ids, + group_ids: group_ids + }.compact + ) + end + + def mutation_result + graphql_mutation_response(:security_scan_profile_attach) + end + + describe 'GraphQL mutation' do + context 'when security_scan_profiles feature is not available' do + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when security_scan_profiles feature is available' do + before do + stub_licensed_features(security_scan_profiles: true) + end + + context 'when user does not have permission' do + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permission' do + before_all do + root_group.add_maintainer(current_user) + end + + context 'when security_scan_profiles feature flag is disabled' do + before do + stub_feature_flags(security_scan_profiles: false) + end + + it 'returns a top-level access error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + end + end + + context 'when security_scan_profiles feature flag is enabled' do + context 'with persisted profile id' do + it 'attaches the profile to projects' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { Security::ScanProfileProject.count }.by(2) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_result['errors']).to be_empty + expect(Security::ScanProfileProject.by_project(project1).by_scan_profile(profile)).to exist + expect(Security::ScanProfileProject.by_project(project2).by_scan_profile(profile)).to exist + end + end + + context 'with template based profile id (scan type)' do + let(:security_scan_profile_id) { 'gid://gitlab/Security::ScanProfile/secret_detection' } + let(:project_ids) { [project1.to_global_id.to_s] } + + context 'when gitlab_recommended profile does not exist' do + it 'creates a new gitlab_recommended profile and attaches it' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { Security::ScanProfile.by_type("secret_detection").gitlab_recommended.count }.by(1) + .and change { Security::ScanProfileProject.count }.by(1) + + expect(response).to have_gitlab_http_status(:success) + scan_profile = Security::ScanProfile.by_type("secret_detection").gitlab_recommended.first + expect(Security::ScanProfileProject.by_project(project1).by_scan_profile(scan_profile)).to exist + end + end + + context 'when gitlab_recommended profile already exists' do + let_it_be(:existing_recommended_profile) do + create(:security_scan_profile, + namespace: root_group, + scan_type: :secret_detection, + gitlab_recommended: true) + end + + it 'does not create a new profile' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .not_to change { Security::ScanProfile.count } + end + + it 'attaches the existing profile' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(Security::ScanProfileProject.by_project(project1) + .by_scan_profile(existing_recommended_profile)).to exist + end + end + end + + context 'with invalid scan type' do + let(:security_scan_profile_id) { 'invalid_type' } + let(:project_ids) { [project1.to_global_id.to_s] } + + it 'returns GraphQL error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + end + end + + context 'with groups in items' do + let(:project_ids) { nil } + let(:group_ids) { [group1.to_global_id.to_s] } + + # it 'enqueues worker for groups' do + # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group1.id, profile.id) + # + # post_graphql_mutation(mutation, current_user: current_user) + # + # expect(response).to have_gitlab_http_status(:success) + # end + + it 'does not create project associations' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .not_to change { Security::ScanProfileProject.count } + end + end + + context 'with mixed groups and projects' do + let(:project_ids) { [project1.to_global_id.to_s] } + let(:group_ids) { [group1.to_global_id.to_s] } + + it 'attaches to projects and enqueues workers for groups' do + # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group1.id, profile.id) + + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { Security::ScanProfileProject.count }.by(1) + + expect(response).to have_gitlab_http_status(:success) + end + end + + context 'when validating arguments' do + context 'when no project_ids or group_ids provided' do + let(:project_ids) { nil } + let(:group_ids) { nil } + + it 'returns GraphQL error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + end + end + + context 'when too many IDs provided' do + let(:project_ids) do + Array.new(101) { project1.to_global_id.to_s } + end + + it 'returns validation error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + expect(graphql_errors.first['message']).to include('Too many ids') + end + end + + context 'when items are from different root namespaces' do + let_it_be(:other_project) { create(:project, namespace: another_root_group) } + let(:project_ids) { [project1.to_global_id.to_s, other_project.to_global_id.to_s] } + + before_all do + another_root_group.add_maintainer(current_user) + end + + it 'returns validation error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + expect(graphql_errors.first['message']).to include('same root namespace') + end + end + end + + context 'when profile does not exist' do + let(:security_scan_profile_id) { "gid://gitlab/Security::ScanProfile/#{non_existing_record_id}" } + + it 'returns GraphQL error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + end + end + + context 'when profile is from different root namespace' do + let_it_be(:other_profile) do + create(:security_scan_profile, + namespace: another_root_group, + scan_type: :secret_detection) + end + + let(:security_scan_profile_id) { other_profile.to_global_id.to_s } + + it 'returns GraphQL error' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + end + end + + context 'when user lacks permission for some projects' do + let_it_be(:other_namespace) { create(:group) } + let_it_be(:other_project) { create(:project, namespace: other_namespace) } + let(:project_ids) { [project1.to_global_id.to_s, other_project.to_global_id.to_s] } + + it 'returns GraphQL error and does not attach to any projects' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .not_to change { Security::ScanProfileProject.count } + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_errors).to be_present + end + end + + context 'when using subgroup project' do + let(:project_ids) { [subgroup_project.to_global_id.to_s] } + let(:security_scan_profile_id) { 'gid://gitlab/Security::ScanProfile/secret_detection' } + + it 'uses the root namespace for profile resolution and attaches successfully' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { Security::ScanProfileProject.count }.by(1) + + profile = Security::ScanProfile.last + expect(profile.namespace).to eq(root_group) + end + end + + context 'when profile is already attached to some projects' do + before do + create(:security_scan_profile_project, scan_profile: profile, project: project1) + end + + it 'is idempotent and does not duplicate' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { Security::ScanProfileProject.count }.by(1) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_result['errors']).to be_empty + end + end + + context 'when project has reached profile limit' do + before do + allow(Security::ScanProfileProject).to receive(:project_ids_at_limit).and_return([project1.id]) + end + + it 'returns error for project at limit and attaches to projects not at limit' do + expect { post_graphql_mutation(mutation, current_user: current_user) } + .to change { Security::ScanProfileProject.count }.by(1) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_result['errors']).to include(match(/Project #{project1.id}.*maximum limit/)) + expect(Security::ScanProfileProject.by_project(project2).by_scan_profile(profile)).to exist + end + end + end + end + end + end +end diff --git a/ee/spec/services/security/scan_profiles/attach_service_spec.rb b/ee/spec/services/security/scan_profiles/attach_service_spec.rb new file mode 100644 index 00000000000000..7958a1df501301 --- /dev/null +++ b/ee/spec/services/security/scan_profiles/attach_service_spec.rb @@ -0,0 +1,207 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanProfiles::AttachService, feature_category: :security_asset_inventories do + let_it_be(:root_group) { create(:group) } + let_it_be(:project1) { create(:project, namespace: root_group) } + let_it_be(:project2) { create(:project, namespace: root_group) } + let_it_be(:project3) { create(:project, namespace: root_group) } + let_it_be(:group1) { create(:group, parent: root_group) } + let_it_be(:group2) { create(:group, parent: root_group) } + let_it_be(:profile) do + create(:security_scan_profile, + namespace: root_group, + scan_type: :secret_detection, + name: 'Test Profile') + end + + shared_examples 'returns empty errors' do + it 'returns empty errors' do + result = execute_service + + expect(result[:errors]).to be_empty + end + end + + describe '.execute' do + subject(:execute_service) do + described_class.execute(profile: profile, projects: projects, groups: groups) + end + + context 'when no projects or groups are provided' do + let(:projects) { [] } + let(:groups) { [] } + + it 'returns an error' do + result = execute_service + + expect(result[:errors]).to include('At least one project or group must be provided') + end + + it 'does not create any associations' do + expect { execute_service }.not_to change { Security::ScanProfileProject.count } + end + end + + context 'when only projects are provided' do + let(:projects) { [project1, project2] } + let(:groups) { [] } + + it 'creates associations for all projects' do + expect { execute_service }.to change { Security::ScanProfileProject.count }.by(projects.count) + end + + it 'creates correct associations' do + execute_service + + expect(Security::ScanProfileProject.where(project: project1, scan_profile: profile)).to exist + expect(Security::ScanProfileProject.where(project: project2, scan_profile: profile)).to exist + end + + it_behaves_like 'returns empty errors' + end + + context 'when only groups are provided' do + let(:projects) { [] } + let(:groups) { [group1, group2] } + + # it 'enqueues workers for each group' do + # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group1.id, profile.id) + # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group2.id, profile.id) + # + # execute_service + # end + + it_behaves_like 'returns empty errors' + end + + context 'when both projects and groups are provided' do + let(:projects) { [project1] } + let(:groups) { [group1] } + + it 'creates associations for projects' do + expect { execute_service }.to change { Security::ScanProfileProject.count }.by(projects.count) + end + + # it 'enqueues workers for groups' do + # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group1.id, profile.id) + # + # execute_service + # end + + it_behaves_like 'returns empty errors' + end + + context 'when a project has reached the profile limit' do + let(:projects) { [project1, project2] } + let(:groups) { [] } + + before do + allow(Security::ScanProfileProject).to receive(:project_ids_at_limit).and_return([project1.id]) + end + + it 'does not attach to the project at limit' do + expect { execute_service }.to change { Security::ScanProfileProject.count }.by(1) + end + + it 'attaches to projects not at limit' do + execute_service + + expect(Security::ScanProfileProject.where(project: project2, scan_profile: profile)).to exist + expect(Security::ScanProfileProject.where(project: project1, scan_profile: profile)).not_to exist + end + + it 'returns an error for the project at limit' do + result = execute_service + + expect(result[:errors]).to include("Project #{project1.id} has reached the maximum limit of scan profiles") + end + end + + context 'when multiple projects have reached the limit' do + let(:projects) { [project1, project2, project3] } + let(:groups) { [] } + + before do + allow(Security::ScanProfileProject).to receive(:project_ids_at_limit).and_return([project1.id, project2.id]) + end + + it 'only attaches to projects not at limit' do + expect { execute_service }.to change { Security::ScanProfileProject.count }.by(1) + end + + it 'attaches only to project3' do + execute_service + + expect(Security::ScanProfileProject.where(project: project3, scan_profile: profile)).to exist + expect(Security::ScanProfileProject.where(project: project1, scan_profile: profile)).not_to exist + expect(Security::ScanProfileProject.where(project: project2, scan_profile: profile)).not_to exist + end + + it 'returns errors for all projects at limit' do + result = execute_service + + expect(result[:errors]).to include("Project #{project1.id} has reached the maximum limit of scan profiles") + expect(result[:errors]).to include("Project #{project2.id} has reached the maximum limit of scan profiles") + end + end + + context 'when profile is already attached to a project' do + let(:projects) { [project1, project2] } + let(:groups) { [] } + + before do + create(:security_scan_profile_project, scan_profile: profile, project: project1) + end + + it 'attaches to projects not yet attached' do + expect { execute_service }.to change { Security::ScanProfileProject.count }.by(1) + + expect(Security::ScanProfileProject.where(project: project2, scan_profile: profile)).to exist + end + + it_behaves_like 'returns empty errors' + end + + context 'when all projects already have the profile attached' do + let(:projects) { [project1, project2] } + let(:groups) { [] } + + before do + create(:security_scan_profile_project, scan_profile: profile, project: project1) + create(:security_scan_profile_project, scan_profile: profile, project: project2) + end + + it 'does not create duplicate associations' do + expect { execute_service }.not_to change { Security::ScanProfileProject.count } + end + + it_behaves_like 'returns empty errors' + end + + context 'when some projects are at limit and some already attached' do + let(:projects) { [project1, project2, project3] } + let(:groups) { [] } + + before do + # project1: already attached + create(:security_scan_profile_project, scan_profile: profile, project: project1) + + # project2: at limit + allow(Security::ScanProfileProject).to receive(:project_ids_at_limit).and_return([project2.id]) + end + + it 'only attaches to project3' do + expect { execute_service }.to change { Security::ScanProfileProject.count }.by(1) + end + + it 'returns error only for project at limit' do + result = execute_service + + expect(result[:errors]).to include("Project #{project2.id} has reached the maximum limit of scan profiles") + expect(result[:errors].size).to eq(1) + end + end + end +end diff --git a/ee/spec/services/security/scan_profiles/find_or_create_service_spec.rb b/ee/spec/services/security/scan_profiles/find_or_create_service_spec.rb new file mode 100644 index 00000000000000..93d457e3ee75d9 --- /dev/null +++ b/ee/spec/services/security/scan_profiles/find_or_create_service_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Security::ScanProfiles::FindOrCreateService, feature_category: :security_asset_inventories do + let_it_be(:root_group) { create(:group) } + let_it_be(:subgroup) { create(:group, parent: root_group) } + let_it_be(:user) { create(:user) } + let_it_be(:another_root_group) { create(:group) } + + describe '.execute' do + subject(:execute_service) do + described_class.execute(namespace: namespace, scan_type: scan_type) + end + + context 'when namespace is not a root namespace' do + let(:namespace) { subgroup } + let(:scan_type) { :secret_detection } + + it 'returns an error' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Namespace must be a root namespace') + end + + it 'does not create a scan profile' do + expect { execute_service }.not_to change { Security::ScanProfile.count } + end + end + + context 'when namespace is a root namespace' do + let(:namespace) { root_group } + + context 'with valid scan_type' do + let(:scan_type) { :secret_detection } + + context 'when no gitlab_recommended profile exists' do + it 'creates a new gitlab_recommended profile' do + expect { execute_service }.to change { Security::ScanProfile.count }.by(1) + end + + it 'returns success with the created profile' do + result = execute_service + + expect(result).to be_success + expect(result.payload[:scan_profile]).to be_a(Security::ScanProfile) + expect(result.payload[:scan_profile]).to be_persisted + end + + it 'creates profile with default attributes' do + result = execute_service + profile = result.payload[:scan_profile] + default_profile = Security::DefaultScanProfilesHelper.default_scan_profiles + .find { |p| p.scan_type == scan_type.to_s } + + expect(profile).to have_attributes( + scan_type: default_profile.scan_type, + name: default_profile.name, + description: default_profile.description, + gitlab_recommended: true, + namespace: namespace + ) + end + end + + context 'when a gitlab_recommended profile already exists' do + let_it_be(:existing_profile) do + create(:security_scan_profile, + namespace: root_group, + scan_type: :secret_detection, + name: 'Existing Secret Detection (default)', + gitlab_recommended: true + ) + end + + it 'does not create a new profile' do + expect { execute_service }.not_to change { Security::ScanProfile.count } + end + + it 'returns the existing profile' do + result = execute_service + + expect(result).to be_success + expect(result.payload[:scan_profile]).to eq(existing_profile) + end + end + + context 'when custom profiles exist but no gitlab_recommended' do + let_it_be(:custom_profile) do + create(:security_scan_profile, + namespace: root_group, + scan_type: :secret_detection, + name: 'Custom Secret Detection', + gitlab_recommended: false + ) + end + + it 'creates a new gitlab_recommended profile' do + expect { execute_service }.to change { Security::ScanProfile.count }.by(1) + end + + it 'returns the newly created gitlab_recommended profile' do + result = execute_service + profile = result.payload[:scan_profile] + + expect(profile).not_to eq(custom_profile) + expect(profile.gitlab_recommended).to be(true) + end + end + end + + context 'with invalid scan_type' do + let(:scan_type) { :invalid_type } + + it 'returns an error' do + result = execute_service + + expect(result).to be_error + expect(result.message).to eq('Could not find a default scan profile for this type') + end + + it 'does not create a scan profile' do + expect { execute_service }.not_to change { Security::ScanProfile.count } + end + end + end + end +end diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 3ddd9a55fac147..beaf0cdd089a48 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -622,6 +622,16 @@ end end + describe '.root_namespace_ids' do + it 'returns unique root namespace ids' do + expect(described_class.where(id: [namespace1.id, namespace1sub.id]).root_namespace_ids).to match_array([namespace1.id]) + expect(described_class.where(id: [namespace2.id, namespace2sub.id]).root_namespace_ids).to match_array([namespace2.id]) + expect(described_class.where(id: [namespace1sub.id, namespace2sub.id]).root_namespace_ids).to match_array([namespace1.id, namespace2.id]) + expect(described_class.where(id: namespace1sub.id).root_namespace_ids).to eq([namespace1.id]) + expect(described_class.where(id: nil).root_namespace_ids).to be_empty + end + end + describe '.filter_by_path' do it 'includes correct namespaces' do expect(described_class.filter_by_path(namespace1.path)).to eq([namespace1]) diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 51b7cf0955f162..e8bb00ae00e0d0 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2951,6 +2951,22 @@ def create_project_statistics_with_size(project, size) end end + describe '.root_namespace_ids' do + let_it_be(:group1) { create(:group) } + let_it_be(:group2) { create(:group) } + let_it_be(:group1_project) { create(:project, namespace: group1) } + let_it_be(:group2_project) { create(:project, namespace: group2) } + let_it_be(:subgroup) { create(:group, parent: group1) } + let_it_be(:subgroup_project) { create(:project, namespace: subgroup) } + + it 'returns unique root namespace ids' do + expect(described_class.where(id: [group1_project.id, subgroup_project.id]).root_namespace_ids).to match_array([group1.id]) + expect(described_class.where(id: group2_project.id).root_namespace_ids).to eq([group2.id]) + expect(described_class.where(id: subgroup_project.id).root_namespace_ids).to eq([group1.id]) + expect(described_class.where(id: nil).root_namespace_ids).to be_empty + end + end + describe '.order_by_storage_size' do let_it_be(:project_1) { create(:project_statistics, repository_size: 1).project } let_it_be(:project_2) { create(:project_statistics, repository_size: 3).project } -- GitLab From bebf66237ceb3b2232a6b49f44048a2a926c6de7 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 5 Dec 2025 13:48:39 -0500 Subject: [PATCH 02/10] Fix CR comments and add MR url --- app/models/namespace.rb | 6 +++++- app/models/project.rb | 2 +- doc/user/custom_roles/abilities.md | 2 +- ee/app/services/security/scan_profiles/attach_service.rb | 1 - ee/config/custom_abilities/apply_security_scan_profiles.yml | 2 +- ee/config/feature_flags/wip/security_scan_profiles.yml | 2 +- 6 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 7cc7912bd2710b..7ded8f2ccb3707 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -230,7 +230,11 @@ class Namespace < ApplicationRecord scope :by_parent, ->(parent) { where(parent_id: parent) } scope :by_root_id, ->(root_id) { where('traversal_ids[1] IN (?)', root_id) } scope :by_not_in_root_id, ->(root_id) { where('namespaces.traversal_ids[1] NOT IN (?)', root_id) } - scope :root_namespace_ids, -> { limit(MAX_PLUCK).pluck(Arel.sql('traversal_ids[1]')).uniq } + scope :root_namespace_ids, -> do + # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- False positive: MAX_PLUCK ensures we have a limit + limit(MAX_PLUCK).distinct.pluck(Arel.sql('traversal_ids[1]')) + # rubocop:enable Database/AvoidUsingPluckWithoutLimit + end scope :filter_by_path, ->(query) { where('lower(path) = :query', query: query.downcase) } scope :in_organization, ->(organization) { where(organization: organization) } scope :by_name, ->(name) { where('name LIKE ?', "#{sanitize_sql_like(name)}%") } diff --git a/app/models/project.rb b/app/models/project.rb index 012b5fb2ea2e87..ea4ad83c3dfc92 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -678,7 +678,7 @@ def with_developer_access scope :by_not_in_root_id, ->(root_id) { joins(:project_namespace).where('namespaces.traversal_ids[1] NOT IN (?)', root_id) } scope :with_visibility_level_greater_than, ->(level) { where("visibility_level > ?", level) } scope :root_namespace_ids, -> { - joins(:namespace).limit(MAX_PLUCK).pluck(Arel.sql('namespaces.traversal_ids[1]')).uniq + joins(:namespace).limit(MAX_PLUCK).distinct.pluck(Arel.sql('namespaces.traversal_ids[1]')) } scope :aimed_for_deletion, -> { where.not(marked_for_deletion_at: nil).without_deleted } diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index bc494482727e0f..9d4482e21461f7 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -102,7 +102,7 @@ Any dependencies are noted in the `Description` column for each permission. | Permission | Description | API Attribute | Scope | Introduced | |:-----------|:------------|:--------------|:------|:-----------| -| Apply security scan profiles | Apply security scan profiles. | [`apply_security_scan_profiles`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/999999999) | Group,
Project | GitLab [18.7](https://gitlab.com/groups/gitlab-org/-/epics/19802) | +| Apply security scan profiles | Apply security scan profiles. | [`apply_security_scan_profiles`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215433) | Group,
Project | GitLab [18.7](https://gitlab.com/groups/gitlab-org/-/epics/19802) | | Read security scan profiles | Read security scan profiles. | [`read_security_scan_profiles`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/213203) | Group,
Project | GitLab [18.7](https://gitlab.com/groups/gitlab-org/-/epics/19802) | ## Security policy management diff --git a/ee/app/services/security/scan_profiles/attach_service.rb b/ee/app/services/security/scan_profiles/attach_service.rb index 18b4779272a1e2..11d14f0f9c2957 100644 --- a/ee/app/services/security/scan_profiles/attach_service.rb +++ b/ee/app/services/security/scan_profiles/attach_service.rb @@ -58,7 +58,6 @@ def bulk_attach_to_projects(projects_to_attach) end Security::ScanProfileProject.upsert_all(records, unique_by: [:project_id, :security_scan_profile_id]) - groups end def attach_to_groups diff --git a/ee/config/custom_abilities/apply_security_scan_profiles.yml b/ee/config/custom_abilities/apply_security_scan_profiles.yml index ef3483f1360b30..aacb99e5c0b557 100644 --- a/ee/config/custom_abilities/apply_security_scan_profiles.yml +++ b/ee/config/custom_abilities/apply_security_scan_profiles.yml @@ -3,7 +3,7 @@ title: Apply security scan profiles name: apply_security_scan_profiles description: Apply security scan profiles. introduced_by_issue: https://gitlab.com/groups/gitlab-org/-/epics/19802 -introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/999999999 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215433 feature_category: security_asset_inventories milestone: '18.7' group_ability: true diff --git a/ee/config/feature_flags/wip/security_scan_profiles.yml b/ee/config/feature_flags/wip/security_scan_profiles.yml index 6165d6e16943ee..310be279ba6024 100644 --- a/ee/config/feature_flags/wip/security_scan_profiles.yml +++ b/ee/config/feature_flags/wip/security_scan_profiles.yml @@ -2,7 +2,7 @@ name: security_scan_profiles description: feature_issue_url: -introduced_by_url: +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215433 rollout_issue_url: milestone: '18.7' group: group::security platform management -- GitLab From 5b26e2cf73d0647ba7d6477d592d7ae7b1603c5d Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 5 Dec 2025 13:48:53 -0500 Subject: [PATCH 03/10] Compile oai docs --- doc/api/openapi/openapi_v2.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/api/openapi/openapi_v2.yaml b/doc/api/openapi/openapi_v2.yaml index 8d26fce8f89d01..f5dfbd3ab03d02 100644 --- a/doc/api/openapi/openapi_v2.yaml +++ b/doc/api/openapi/openapi_v2.yaml @@ -53009,6 +53009,8 @@ definitions: - 30 - 40 example: 40 + apply_security_scan_profiles: + type: boolean admin_merge_request: type: boolean archive_project: -- GitLab From 72123b58ee17c65ed6ddf46ddef0d8cbf95be904 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 5 Dec 2025 13:53:17 -0500 Subject: [PATCH 04/10] Change feature flag name to avoid conflicts --- ee/app/graphql/mutations/security/scan_profiles/attach.rb | 2 +- ...scan_profiles.yml => security_scan_profiles_feature.yml} | 2 +- .../graphql/mutations/security/scan_profiles/attach_spec.rb | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) rename ee/config/feature_flags/wip/{security_scan_profiles.yml => security_scan_profiles_feature.yml} (86%) diff --git a/ee/app/graphql/mutations/security/scan_profiles/attach.rb b/ee/app/graphql/mutations/security/scan_profiles/attach.rb index 954ecd197fccc9..d576923243e2fb 100644 --- a/ee/app/graphql/mutations/security/scan_profiles/attach.rb +++ b/ee/app/graphql/mutations/security/scan_profiles/attach.rb @@ -26,7 +26,7 @@ def resolve(security_scan_profile_id:, project_ids: nil, group_ids: nil) validate_id_limit!(project_ids, group_ids) root_namespace = shared_root_namespace!(project_ids, group_ids) - raise_resource_not_available_error! unless Feature.enabled?(:security_scan_profiles, root_namespace) + raise_resource_not_available_error! unless Feature.enabled?(:security_scan_profiles_feature, root_namespace) authorized_projects = load_and_authorize_projects(project_ids) authorized_groups = load_and_authorize_groups(group_ids) diff --git a/ee/config/feature_flags/wip/security_scan_profiles.yml b/ee/config/feature_flags/wip/security_scan_profiles_feature.yml similarity index 86% rename from ee/config/feature_flags/wip/security_scan_profiles.yml rename to ee/config/feature_flags/wip/security_scan_profiles_feature.yml index 310be279ba6024..5522daf4602539 100644 --- a/ee/config/feature_flags/wip/security_scan_profiles.yml +++ b/ee/config/feature_flags/wip/security_scan_profiles_feature.yml @@ -1,5 +1,5 @@ --- -name: security_scan_profiles +name: security_scan_profiles_feature description: feature_issue_url: introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/215433 diff --git a/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb b/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb index 7b60b20feec606..0bed1cb1af5f1f 100644 --- a/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb @@ -58,9 +58,9 @@ def mutation_result root_group.add_maintainer(current_user) end - context 'when security_scan_profiles feature flag is disabled' do + context 'when security_scan_profiles_feature feature flag is disabled' do before do - stub_feature_flags(security_scan_profiles: false) + stub_feature_flags(security_scan_profiles_feature: false) end it 'returns a top-level access error' do @@ -71,7 +71,7 @@ def mutation_result end end - context 'when security_scan_profiles feature flag is enabled' do + context 'when security_scan_profiles_feature feature flag is enabled' do context 'with persisted profile id' do it 'attaches the profile to projects' do expect { post_graphql_mutation(mutation, current_user: current_user) } -- GitLab From c8c20a18672a471cbb5f352510a2e3c2a933ac0f Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 5 Dec 2025 14:31:36 -0500 Subject: [PATCH 05/10] Improve bulk permissions check --- ee/app/graphql/mutations/security/scan_profiles/attach.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ee/app/graphql/mutations/security/scan_profiles/attach.rb b/ee/app/graphql/mutations/security/scan_profiles/attach.rb index d576923243e2fb..9eb3254f0b7f11 100644 --- a/ee/app/graphql/mutations/security/scan_profiles/attach.rb +++ b/ee/app/graphql/mutations/security/scan_profiles/attach.rb @@ -66,13 +66,15 @@ def resolve_profile(security_scan_profile_id, root_namespace) def load_and_authorize_projects(project_ids) return [] unless project_ids&.any? - Project.id_in(project_ids).select { |project| current_user.can?(:apply_security_scan_profiles, project) } + all_project = Project.id_in(project_ids) + Project.projects_user_can(all_project, current_user, :apply_security_scan_profiles) end def load_and_authorize_groups(group_ids) return [] unless group_ids&.any? - Group.id_in(group_ids).select { |group| current_user.can?(:apply_security_scan_profiles, group) } + all_groups = Group.id_in(group_ids) + Group.groups_user_can(all_groups, current_user, :apply_security_scan_profiles) end def validate_id_limit!(project_ids, group_ids) -- GitLab From 0b0570a181000cd97a756310178ecea806473f17 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Wed, 10 Dec 2025 15:52:04 -0500 Subject: [PATCH 06/10] Move group logic to mutation --- .../security/scan_profiles/attach.rb | 20 ++++-- .../security/scan_profiles/attach_service.rb | 32 ++++------ .../scan_profiles/attach_service_spec.rb | 63 ++++++------------- 3 files changed, 46 insertions(+), 69 deletions(-) diff --git a/ee/app/graphql/mutations/security/scan_profiles/attach.rb b/ee/app/graphql/mutations/security/scan_profiles/attach.rb index 9eb3254f0b7f11..9ee6d29a4a1f64 100644 --- a/ee/app/graphql/mutations/security/scan_profiles/attach.rb +++ b/ee/app/graphql/mutations/security/scan_profiles/attach.rb @@ -37,11 +37,8 @@ def resolve(security_scan_profile_id:, project_ids: nil, group_ids: nil) profile = resolve_profile(security_scan_profile_id, root_namespace) raise_resource_not_available_error! unless profile - result = ::Security::ScanProfiles::AttachService.execute( - profile: profile, - projects: authorized_projects, - groups: authorized_groups - ) + result = ::Security::ScanProfiles::AttachService.execute(profile: profile, projects: authorized_projects) + schedule_group_workers(authorized_groups, profile) { errors: result[:errors] @@ -77,6 +74,19 @@ def load_and_authorize_groups(group_ids) Group.groups_user_can(all_groups, current_user, :apply_security_scan_profiles) end + def schedule_group_workers(groups, profile) + return if groups.empty? + + # groups.each do |group| + # SecurityScanProfiles::AttachWorker.perform_async( + # group.id, + # profile.id + # ) + # end + + groups && profile # for rubocop + end + def validate_id_limit!(project_ids, group_ids) total = project_ids.to_a.size + group_ids.to_a.size return if total <= MAX_IDS diff --git a/ee/app/services/security/scan_profiles/attach_service.rb b/ee/app/services/security/scan_profiles/attach_service.rb index 11d14f0f9c2957..b1e0965913af88 100644 --- a/ee/app/services/security/scan_profiles/attach_service.rb +++ b/ee/app/services/security/scan_profiles/attach_service.rb @@ -3,22 +3,27 @@ module Security module ScanProfiles class AttachService - def self.execute(profile:, projects: [], groups: []) - new(profile: profile, projects: projects, groups: groups).execute + MAX_PROJECTS = 500 + TooManyProjectsError = Class.new(StandardError) + + def self.execute(profile:, projects: []) + new(profile: profile, projects: projects).execute end - def initialize(profile:, projects: [], groups: []) + def initialize(profile:, projects: []) + if projects.size > MAX_PROJECTS + raise TooManyProjectsError, "Cannot attach profile to more than #{MAX_PROJECTS} items at once." + end + @profile = profile @projects = projects - @groups = groups @errors = [] end def execute - return error_result('At least one project or group must be provided') if projects.empty? && groups.empty? + return error_result('At least one project must be provided') if projects.empty? attach_to_projects - attach_to_groups { errors: errors @@ -27,7 +32,7 @@ def execute error_result(e.message) end - attr_reader :profile, :projects, :groups, :errors + attr_reader :profile, :projects, :errors private @@ -60,19 +65,6 @@ def bulk_attach_to_projects(projects_to_attach) Security::ScanProfileProject.upsert_all(records, unique_by: [:project_id, :security_scan_profile_id]) end - def attach_to_groups - return [] if groups.empty? - - # groups.each do |group| - # SecurityScanProfiles::AttachWorker.perform_async( - # group.id, - # profile.id - # ) - # end - - groups # REMOVE - This is here so rubocop won't be mad - end - def error_result(message) { errors: [message] diff --git a/ee/spec/services/security/scan_profiles/attach_service_spec.rb b/ee/spec/services/security/scan_profiles/attach_service_spec.rb index 7958a1df501301..a10928c960f337 100644 --- a/ee/spec/services/security/scan_profiles/attach_service_spec.rb +++ b/ee/spec/services/security/scan_profiles/attach_service_spec.rb @@ -7,8 +7,6 @@ let_it_be(:project1) { create(:project, namespace: root_group) } let_it_be(:project2) { create(:project, namespace: root_group) } let_it_be(:project3) { create(:project, namespace: root_group) } - let_it_be(:group1) { create(:group, parent: root_group) } - let_it_be(:group2) { create(:group, parent: root_group) } let_it_be(:profile) do create(:security_scan_profile, namespace: root_group, @@ -26,17 +24,16 @@ describe '.execute' do subject(:execute_service) do - described_class.execute(profile: profile, projects: projects, groups: groups) + described_class.execute(profile: profile, projects: projects) end - context 'when no projects or groups are provided' do + context 'when no projects are provided' do let(:projects) { [] } - let(:groups) { [] } it 'returns an error' do result = execute_service - expect(result[:errors]).to include('At least one project or group must be provided') + expect(result[:errors]).to include('At least one project must be provided') end it 'does not create any associations' do @@ -44,9 +41,8 @@ end end - context 'when only projects are provided' do + context 'when projects are provided' do let(:projects) { [project1, project2] } - let(:groups) { [] } it 'creates associations for all projects' do expect { execute_service }.to change { Security::ScanProfileProject.count }.by(projects.count) @@ -62,40 +58,8 @@ it_behaves_like 'returns empty errors' end - context 'when only groups are provided' do - let(:projects) { [] } - let(:groups) { [group1, group2] } - - # it 'enqueues workers for each group' do - # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group1.id, profile.id) - # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group2.id, profile.id) - # - # execute_service - # end - - it_behaves_like 'returns empty errors' - end - - context 'when both projects and groups are provided' do - let(:projects) { [project1] } - let(:groups) { [group1] } - - it 'creates associations for projects' do - expect { execute_service }.to change { Security::ScanProfileProject.count }.by(projects.count) - end - - # it 'enqueues workers for groups' do - # expect(SecurityScanProfiles::AttachWorker).to receive(:perform_async).with(group1.id, profile.id) - # - # execute_service - # end - - it_behaves_like 'returns empty errors' - end - context 'when a project has reached the profile limit' do let(:projects) { [project1, project2] } - let(:groups) { [] } before do allow(Security::ScanProfileProject).to receive(:project_ids_at_limit).and_return([project1.id]) @@ -121,7 +85,6 @@ context 'when multiple projects have reached the limit' do let(:projects) { [project1, project2, project3] } - let(:groups) { [] } before do allow(Security::ScanProfileProject).to receive(:project_ids_at_limit).and_return([project1.id, project2.id]) @@ -149,7 +112,6 @@ context 'when profile is already attached to a project' do let(:projects) { [project1, project2] } - let(:groups) { [] } before do create(:security_scan_profile_project, scan_profile: profile, project: project1) @@ -166,7 +128,6 @@ context 'when all projects already have the profile attached' do let(:projects) { [project1, project2] } - let(:groups) { [] } before do create(:security_scan_profile_project, scan_profile: profile, project: project1) @@ -182,7 +143,6 @@ context 'when some projects are at limit and some already attached' do let(:projects) { [project1, project2, project3] } - let(:groups) { [] } before do # project1: already attached @@ -203,5 +163,20 @@ expect(result[:errors].size).to eq(1) end end + + context 'when more than MAX_PROJECTS are provided' do + let(:projects) { [project1, project2, project3] } + + before do + stub_const("#{described_class}::MAX_PROJECTS", 2) + end + + it 'raises TooManyProjectsError' do + expect { execute_service }.to raise_error do |error| + expect(error.class).to eql(described_class::TooManyProjectsError) + expect(error.message).to eql('Cannot attach profile to more than 2 items at once.') + end + end + end end end -- GitLab From 1562dff591fb4253046d334b1106e1da7ad771a9 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Thu, 11 Dec 2025 09:40:37 -0500 Subject: [PATCH 07/10] Fix non chainable scopes --- app/models/namespace.rb | 11 ++++--- app/models/project.rb | 7 ++-- .../models/security/scan_profile_project.rb | 5 +-- .../security/scan_profile_project_spec.rb | 7 ++-- spec/models/namespace_spec.rb | 25 +++++++++------ spec/models/project_spec.rb | 32 +++++++++---------- 6 files changed, 49 insertions(+), 38 deletions(-) diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 7ded8f2ccb3707..48cad7f12f648e 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -230,11 +230,6 @@ class Namespace < ApplicationRecord scope :by_parent, ->(parent) { where(parent_id: parent) } scope :by_root_id, ->(root_id) { where('traversal_ids[1] IN (?)', root_id) } scope :by_not_in_root_id, ->(root_id) { where('namespaces.traversal_ids[1] NOT IN (?)', root_id) } - scope :root_namespace_ids, -> do - # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- False positive: MAX_PLUCK ensures we have a limit - limit(MAX_PLUCK).distinct.pluck(Arel.sql('traversal_ids[1]')) - # rubocop:enable Database/AvoidUsingPluckWithoutLimit - end scope :filter_by_path, ->(query) { where('lower(path) = :query', query: query.downcase) } scope :in_organization, ->(organization) { where(organization: organization) } scope :by_name, ->(name) { where('name LIKE ?', "#{sanitize_sql_like(name)}%") } @@ -417,6 +412,12 @@ def self_or_ancestors_archived_setting_subquery ) .where(namespace_setting_table[:archived].eq(true)) end + + def root_namespace_ids + # rubocop:disable Database/AvoidUsingPluckWithoutLimit -- False positive: MAX_PLUCK ensures we have a limit + limit(Namespace::MAX_PLUCK).distinct.pluck(Arel.sql('traversal_ids[1]')) + # rubocop:enable Database/AvoidUsingPluckWithoutLimit + end end def archived? diff --git a/app/models/project.rb b/app/models/project.rb index ea4ad83c3dfc92..1a0f5722e046e4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -677,9 +677,6 @@ def with_developer_access scope :not_in_groups, ->(groups) { where.not(group: groups) } scope :by_not_in_root_id, ->(root_id) { joins(:project_namespace).where('namespaces.traversal_ids[1] NOT IN (?)', root_id) } scope :with_visibility_level_greater_than, ->(level) { where("visibility_level > ?", level) } - scope :root_namespace_ids, -> { - joins(:namespace).limit(MAX_PLUCK).distinct.pluck(Arel.sql('namespaces.traversal_ids[1]')) - } scope :aimed_for_deletion, -> { where.not(marked_for_deletion_at: nil).without_deleted } scope :self_or_ancestors_aimed_for_deletion, -> do @@ -1347,6 +1344,10 @@ def group_by_namespace_traversal_ids(project_batch) .group_by(&:last) .transform_values { |projects| projects.map(&:first) } end + + def root_namespace_ids + joins(:namespace).limit(Project::MAX_PLUCK).distinct.pluck(Arel.sql('namespaces.traversal_ids[1]')) + end end def initialize(attributes = nil) diff --git a/ee/app/models/security/scan_profile_project.rb b/ee/app/models/security/scan_profile_project.rb index 82872c30b5cab9..44448bf4804c0d 100644 --- a/ee/app/models/security/scan_profile_project.rb +++ b/ee/app/models/security/scan_profile_project.rb @@ -17,9 +17,10 @@ class ScanProfileProject < ::SecApplicationRecord } scope :by_scan_profile, ->(profile_id) { where(security_scan_profile_id: profile_id) } scope :by_project, ->(project_ids) { where(project_id: project_ids) } - scope :project_ids_at_limit, -> { + + def self.project_ids_at_limit group(:project_id).having('COUNT(*) >= ?', MAX_PROFILES_PER_PROJECT) .limit(MAX_PLUCK).pluck(:project_id) - } + end end end diff --git a/ee/spec/models/security/scan_profile_project_spec.rb b/ee/spec/models/security/scan_profile_project_spec.rb index bfe1da31bdd0c1..ca98b213529d12 100644 --- a/ee/spec/models/security/scan_profile_project_spec.rb +++ b/ee/spec/models/security/scan_profile_project_spec.rb @@ -73,10 +73,13 @@ expect(described_class.by_project(nil)).to be_empty end end + end + describe 'class methods' do describe '.project_ids_at_limit' do - let_it_be(:project_at_limit) { create(:project, namespace: root_namespace) } - let_it_be(:project_under_limit) { create(:project, namespace: root_namespace) } + let_it_be(:project_at_limit) { create(:project, namespace: group) } + let_it_be(:project_under_limit) { create(:project, namespace: group) } + let_it_be(:scan_profile2) { create(:security_scan_profile, namespace: group, name: 'Profile 2') } before do stub_const("#{described_class}::MAX_PROFILES_PER_PROJECT", 2) diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index beaf0cdd089a48..aea320f4cd2c34 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -622,16 +622,6 @@ end end - describe '.root_namespace_ids' do - it 'returns unique root namespace ids' do - expect(described_class.where(id: [namespace1.id, namespace1sub.id]).root_namespace_ids).to match_array([namespace1.id]) - expect(described_class.where(id: [namespace2.id, namespace2sub.id]).root_namespace_ids).to match_array([namespace2.id]) - expect(described_class.where(id: [namespace1sub.id, namespace2sub.id]).root_namespace_ids).to match_array([namespace1.id, namespace2.id]) - expect(described_class.where(id: namespace1sub.id).root_namespace_ids).to eq([namespace1.id]) - expect(described_class.where(id: nil).root_namespace_ids).to be_empty - end - end - describe '.filter_by_path' do it 'includes correct namespaces' do expect(described_class.filter_by_path(namespace1.path)).to eq([namespace1]) @@ -2028,6 +2018,21 @@ end end + describe '.root_namespace_ids' do + let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') } + let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') } + let_it_be(:namespace1sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace1) } + let_it_be(:namespace2sub) { create(:group, name: 'Sub Namespace', path: 'sub-namespace', parent: namespace2) } + + it 'returns unique root namespace ids' do + expect(described_class.id_in([namespace1.id, namespace1sub.id]).root_namespace_ids).to match_array([namespace1.id]) + expect(described_class.id_in([namespace2.id, namespace2sub.id]).root_namespace_ids).to match_array([namespace2.id]) + expect(described_class.id_in([namespace1sub.id, namespace2sub.id]).root_namespace_ids).to match_array([namespace1.id, namespace2.id]) + expect(described_class.id_in(namespace1sub.id).root_namespace_ids).to eq([namespace1.id]) + expect(described_class.id_in(nil).root_namespace_ids).to be_empty + end + end + shared_examples 'disabled feature flag when traversal_ids is blank' do before do namespace.traversal_ids = [] diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index e8bb00ae00e0d0..787ffc3a9aa65b 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -2951,22 +2951,6 @@ def create_project_statistics_with_size(project, size) end end - describe '.root_namespace_ids' do - let_it_be(:group1) { create(:group) } - let_it_be(:group2) { create(:group) } - let_it_be(:group1_project) { create(:project, namespace: group1) } - let_it_be(:group2_project) { create(:project, namespace: group2) } - let_it_be(:subgroup) { create(:group, parent: group1) } - let_it_be(:subgroup_project) { create(:project, namespace: subgroup) } - - it 'returns unique root namespace ids' do - expect(described_class.where(id: [group1_project.id, subgroup_project.id]).root_namespace_ids).to match_array([group1.id]) - expect(described_class.where(id: group2_project.id).root_namespace_ids).to eq([group2.id]) - expect(described_class.where(id: subgroup_project.id).root_namespace_ids).to eq([group1.id]) - expect(described_class.where(id: nil).root_namespace_ids).to be_empty - end - end - describe '.order_by_storage_size' do let_it_be(:project_1) { create(:project_statistics, repository_size: 1).project } let_it_be(:project_2) { create(:project_statistics, repository_size: 3).project } @@ -3334,6 +3318,22 @@ def create_project_statistics_with_size(project, size) end end + describe '.root_namespace_ids' do + let_it_be(:group1) { create(:group) } + let_it_be(:group2) { create(:group) } + let_it_be(:group1_project) { create(:project, namespace: group1) } + let_it_be(:group2_project) { create(:project, namespace: group2) } + let_it_be(:subgroup) { create(:group, parent: group1) } + let_it_be(:subgroup_project) { create(:project, namespace: subgroup) } + + it 'returns unique root namespace ids' do + expect(described_class.where(id: [group1_project.id, subgroup_project.id]).root_namespace_ids).to match_array([group1.id]) + expect(described_class.where(id: group2_project.id).root_namespace_ids).to eq([group2.id]) + expect(described_class.where(id: subgroup_project.id).root_namespace_ids).to eq([group1.id]) + expect(described_class.where(id: nil).root_namespace_ids).to be_empty + end + end + context 'repository storage by default' do let(:project) { build(:project) } -- GitLab From 5a874ea6a0f3bf0bca8832a31368324f3ce1d8c3 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Thu, 11 Dec 2025 10:58:59 -0500 Subject: [PATCH 08/10] Replace upsert_all with insert_all to improve performance --- ee/app/services/security/scan_profiles/attach_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/app/services/security/scan_profiles/attach_service.rb b/ee/app/services/security/scan_profiles/attach_service.rb index b1e0965913af88..1efbe778e2eb41 100644 --- a/ee/app/services/security/scan_profiles/attach_service.rb +++ b/ee/app/services/security/scan_profiles/attach_service.rb @@ -62,7 +62,7 @@ def bulk_attach_to_projects(projects_to_attach) } end - Security::ScanProfileProject.upsert_all(records, unique_by: [:project_id, :security_scan_profile_id]) + Security::ScanProfileProject.insert_all(records, unique_by: [:project_id, :security_scan_profile_id]) end def error_result(message) -- GitLab From 38e07f856df37d56791bd8cb1b2bb80dcc7f17c4 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 12 Dec 2025 11:12:16 -0500 Subject: [PATCH 09/10] Fix scope usage --- .../services/security/scan_profiles/find_or_create_service.rb | 2 +- ee/spec/models/security/scan_profile_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/ee/app/services/security/scan_profiles/find_or_create_service.rb b/ee/app/services/security/scan_profiles/find_or_create_service.rb index 426eaaee438fda..e95b901668efa7 100644 --- a/ee/app/services/security/scan_profiles/find_or_create_service.rb +++ b/ee/app/services/security/scan_profiles/find_or_create_service.rb @@ -15,7 +15,7 @@ def initialize(namespace:, scan_type:) def execute return error_response('Namespace must be a root namespace') unless namespace.root? - profile = Security::ScanProfile.by_namespace(namespace).by_type(scan_type).gitlab_recommended.first + profile = Security::ScanProfile.by_namespace(namespace).by_type(scan_type).by_gitlab_recommended.first return success_response(profile) if profile profile = create_profile diff --git a/ee/spec/models/security/scan_profile_spec.rb b/ee/spec/models/security/scan_profile_spec.rb index 3fde66d379ae90..b27b73b0d86876 100644 --- a/ee/spec/models/security/scan_profile_spec.rb +++ b/ee/spec/models/security/scan_profile_spec.rb @@ -53,7 +53,7 @@ end describe 'scopes' do - describe '.gitlab_recommended' do + describe '.by_gitlab_recommended' do let_it_be(:profile) { create(:security_scan_profile, namespace: root_level_group, scan_type: :secret_detection) } let_it_be(:gitlab_recommended_profile) do create(:security_scan_profile, @@ -65,7 +65,7 @@ end it 'returns gitlab recommended profiles' do - expect(described_class.gitlab_recommended).to match_array([gitlab_recommended_profile]) + expect(described_class.by_gitlab_recommended).to match_array([gitlab_recommended_profile]) end end end -- GitLab From 6b0db1e02d3f458c28772f0d2311625ffb5ee0d2 Mon Sep 17 00:00:00 2001 From: Gal Katz Date: Fri, 12 Dec 2025 13:16:14 -0500 Subject: [PATCH 10/10] Fix scope usage again --- .../graphql/mutations/security/scan_profiles/attach_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb b/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb index 0bed1cb1af5f1f..2b1eb51d50e0bd 100644 --- a/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb +++ b/ee/spec/requests/api/graphql/mutations/security/scan_profiles/attach_spec.rb @@ -91,11 +91,11 @@ def mutation_result context 'when gitlab_recommended profile does not exist' do it 'creates a new gitlab_recommended profile and attaches it' do expect { post_graphql_mutation(mutation, current_user: current_user) } - .to change { Security::ScanProfile.by_type("secret_detection").gitlab_recommended.count }.by(1) + .to change { Security::ScanProfile.by_type("secret_detection").by_gitlab_recommended.count }.by(1) .and change { Security::ScanProfileProject.count }.by(1) expect(response).to have_gitlab_http_status(:success) - scan_profile = Security::ScanProfile.by_type("secret_detection").gitlab_recommended.first + scan_profile = Security::ScanProfile.by_type("secret_detection").by_gitlab_recommended.first expect(Security::ScanProfileProject.by_project(project1).by_scan_profile(scan_profile)).to exist end end -- GitLab