Skip to content

Block ip-restricted requests from creating pipelines

drew stachon requested to merge add-read-project-to-ci-chain-validate into master

What does this MR do and why?

This MR adds an explicit check of read_project to the Pipeline creation process. The security gap was closed in a prior MR and backported, but we want to add this check to the CreatePipelineService process, where it really belongs.

The follow-up issue articulating this change is https://gitlab.com/gitlab-org/gitlab/-/issues/363745.

Rather than build the check into allowed_to_write_ref?, I added it to allowed_to_create_pipeline?. It seemed more cohesive to write into the existing create-pipeline permission check, which is what we're preventing here. The specific semantics of adding to a write-to-ref check made less sense to me. Either way, both methods are private so that's an implementation detail, and I wrote higher-level specs for the public perform! method, where we detail other specific contexts.

How to set up and validate locally

  1. bundle exec rspec spec/lib/gitlab/ci/pipeline/chain/validate/abilities_spec.rb
  2. Comment out current_user.can?(:read_project, project) && from the first line of Abilities#allowed_to_create_pipeline?
  3. Run the new tests again, they'll fail.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Merge request reports