Follow-up from "Add setting for enabling CS for Registry"
The following discussion from !148242 (merged) should be addressed:
-
@fabiopitino started a discussion: (+1 comment) @atiwari71 I noticed that we would also need to ensure we don't enforce compliance or scan execution policies on those pipelines.
There is already some existing code around this that we need to modify to include the new
container_registry_push
source. Otherwise, on top of the regular Container Scanning, the pipeline could include jobs injected by SEP or Compliance framework.Here below is a suggestion on how we could refactor and fix this:
diff --git a/app/models/concerns/enums/ci/pipeline.rb b/app/models/concerns/enums/ci/pipeline.rb index 7dd9dece5e07..4c95fda3676e 100644 --- a/app/models/concerns/enums/ci/pipeline.rb +++ b/app/models/concerns/enums/ci/pipeline.rb @@ -65,6 +65,22 @@ def self.dangling_sources sources.slice(:webide, :parent_pipeline, :ondemand_dast_scan, :ondemand_dast_validation, :security_orchestration_policy) end + def self.safe_sources + dangling_sources.slice( + :ondemand_dast_scan, + :security_orchestration_policy + ) + end + + def self.enforce_compliance_for_source?(source) + sources.except( + :parent_pipeline, # Child pipelines. Compliance is enforced on the parent pipeline + :security_orchestration_policy, # explain why... + :ondemand_dast_scan, # explain why... + :container_registry_push # explain why... + ).include?(source) + end + # CI sources are those pipeline events that affect the CI status of the ref # they run for. By definition it excludes dangling pipelines. def self.ci_sources diff --git a/ee/lib/gitlab/ci/project_config/compliance.rb b/ee/lib/gitlab/ci/project_config/compliance.rb index 5015b9445fc6..e1771af3d659 100644 --- a/ee/lib/gitlab/ci/project_config/compliance.rb +++ b/ee/lib/gitlab/ci/project_config/compliance.rb @@ -6,11 +6,9 @@ class ProjectConfig class Compliance < Gitlab::Ci::ProjectConfig::Source def content strong_memoize(:content) do + next unless ::Ci::Pipeline.enforce_compliance_for_source?(pipeline_source) next unless available? next unless pipeline_configuration_full_path.present? - next if pipeline_source_bridge && pipeline_source == :parent_pipeline - - next if [:security_orchestration_policy, :ondemand_dast_scan].include?(pipeline_source) path_file, path_project = pipeline_configuration_full_path.split('@', 2) YAML.dump('include' => [{ 'project' => path_project, 'file' => path_file }])
But I'm also noticing that a similar pattern would need to be applied to https://gitlab.com/gitlab-org/gitlab/-/blob/a1c420312e52275e308a9058d0bce9a195c0e8f2/ee/lib/gitlab/ci/project_config/security_policy_default.rb as well. We should not enforce SEP configurations on a
container_registry_push
pipeline (and possibly other ones likeondemand_dast_scan
) if the project doesn't have a.gitlab-ci.yml
./cc @mc_rocha @mcavoj @furkanayhan for awareness.
I would be ok with making this change as a separate MR right after this one and before we enable the feature flag.