Scan Result Policy - `group_approvers` string matches across all namespaces
Summary
If you add compliance
to the group_approvers
in a Scan Result Policy, the approval groups added will match the group name compliance
, however it adds multiple groups from vastly different namespaces.
This issue was discovered by a customer.
Customer is interested in the outcome of this issue, so please change to non-confidential if this is not required. I made this confidential out of an abundance of caution.
Solution proposal
We have implemented an admin feature that allows instance owners to toggle the search to be open to search across namespaces, or closed to restrict searching to within a namespace.
For GitLab.com, we've enabled this feature to address the bug. For self-managed users, this can be used as a feature to restrict searching across namespaces where it may be appropriate.
The setting security_policy_global_group_approvers_enabled
is documented under Application settings API. The page also explains how to Change application settings.
Steps to reproduce
- Create a new project
- Create a new Scan Result Policy. In YAML view, change
group_approvers
so it has["compliance"]
- Save the Scan Result Policy, merge via MR.
- Edit the policy you've just added in the project. Confirm groups added as approvers.
Example Project
https://gitlab.com/gitlab-gold/tmike/zd323145/zd323145-2/zd323145-2/
What is the current bug behavior?
Groups named compliance
are matched but from a lot of different namespaces:
anthonybaer/gitlab-evaluation/compliance
gaia-x/lab/compliance
gitlab-org/govern/compliance
pensando/tbd/compliance
gitlab-gold/briecarranza/tickets/compliance
blee-group/demos/security/compliance
What is the expected correct behavior?
Typically users for self managed instances want to preserve the current functionality; however, for shared instances such as GitLab SaaS, this is viewed as a bug.
This should be an instance-level Application Setting to control whether or not the search extends across all namespaces. The setting should default to the current behavior (searching all namespaces). Once this new setting is introduced, we will change the setting for GitLab SaaS to the non-default value to prevent searching outside of the current namespace.
Relevant logs and/or screenshots
Output of checks
Results of GitLab environment info
Expand for output related to GitLab environment info
(For installations with omnibus-gitlab package run and paste the output of: `sudo gitlab-rake gitlab:env:info`) (For installations from source run and paste the output of: `sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)
Results of GitLab application Check
Expand for output related to the GitLab application check
(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true
)(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true
)(we will only investigate if the tests are passing)
Implementation plan
-
backend Create a new application_setting security_policy_global_group_approvers_enabled
and make it disabled by default for gitlab.com
class AddGlobalGroupApproversEnabledToApplicationSettings < Gitlab::Database::Migration[2.0]
def change
add_column :application_settings, :security_policy_global_group_approvers_enabled, :boolean, default: false, null: false
end
end
-
backend Update Security::SecurityOrchestrationPolicies::FetchPolicyApproversService
to filter the groups only through the root namespace ifsecurity_policy_global_group_approvers_enabled
is false
diff --git a/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb b/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb
index 7f23f91a070b..3e968eeb3df0 100644
--- a/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb
+++ b/ee/app/services/security/security_orchestration_policies/fetch_policy_approvers_service.rb
@@ -50,7 +50,14 @@ def group_approvers(action)
# Using GroupFinder here would make groups more restrictive than current features related to others approval project rules as in:
# https://gitlab.com/gitlab-org/gitlab/-/blob/0aa924eaa1a4ca5ed6b226d826f7298ec847ea5f/ee/app/services/concerns/approval_rules/updater.rb#L44
# Therefore data migrated from Vulnerability-Check into Scan result policies would be inconsistent.
- Group.public_or_visible_to_user(current_user).by_ids_or_paths(group_ids, group_paths)
+
+ if (Gitlab::CurrentSettings.security_policy_global_group_approvers_enabled)
+ Group.public_or_visible_to_user(current_user).by_ids_or_paths(group_ids, group_paths)
+ else
+ container.root_ancestor.self_and_descendants.select(:id)
+ .public_or_visible_to_user(current_user)
+ .by_ids_or_paths(group_ids, group_paths)
+ end
end
# rubocop: enable Cop/GroupPublicOrVisibleToUser
-
backend Pass the setting value and root_ancestor
of the project or group to the frontend in ee/app/helpers/ee/security_orchestration_helper.rb
diff --git a/ee/app/helpers/ee/security_orchestration_helper.rb b/ee/app/helpers/ee/security_orchestration_helper.rb
index a4cb77d573f6..b9c37eb86bba 100644
--- a/ee/app/helpers/ee/security_orchestration_helper.rb
+++ b/ee/app/helpers/ee/security_orchestration_helper.rb
@@ -34,7 +34,9 @@ def orchestration_policy_data(container, policy_type = nil, policy = nil, approv
policy_editor_empty_state_svg_path: image_path('illustrations/monitoring/unable_to_connect.svg'),
policy_type: policy_type,
scan_policy_documentation_path: help_page_path('user/application_security/policies/index'),
- scan_result_approvers: approvers&.to_json
+ scan_result_approvers: approvers&.to_json,
+ global_group_approvers_enabled: Gitlab::CurrentSettings.security_policy_global_group_approvers_enabled,
+ root_namespace_path: container.root_ancestor&.full_path
}
if container.is_a?(::Project)
-
backend Create grapqhl query equivalent of List Groups API -
frontend Update the frontend component to use the new graphql groups query if global_group_approvers_enabled
is true andGroup.descendantGroups
ifglobal_group_approvers_enabled
is false. We can useroot_namespace_path
fromorchestration_policy_data
to search the groups. (Search only descendant groups when applicable (!113625 - merged))
query {
group(fullPath:$rootNamespacePath){
descendantGroups(search:$searchTerm) {
nodes{
name
}
}
}
}